Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Substitute this with self #3524

Merged
merged 13 commits into from
Jun 21, 2022
Merged

Substitute this with self #3524

merged 13 commits into from
Jun 21, 2022

Conversation

hubertp
Copy link
Contributor

@hubertp hubertp commented Jun 13, 2022

Pull Request Description

A semi-manual s/this/self appied to the whole standard library.
Related to https://www.pivotaltracker.com/story/show/182328601

In the compiler promoted to use constants instead of hardcoded
this/self whenever possible.

Important Notes

The PR does not require explicit self parameter declaration for methods as this part
of the design is still under consideration.

Checklist

Please include the following checklist in your PR:

  • The documentation has been updated if necessary.
  • All code conforms to the
    Scala,
    Java,
    and
    Rust
    style guides.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed: Enso GUI was tested when built using BOTH
      ./run ide dist and ./run ide watch.

@hubertp
Copy link
Contributor Author

hubertp commented Jun 13, 2022

I dare anyone to review distribution/lib. I double dare you ;)

@hubertp hubertp force-pushed the wip/hubert/this-to-self-182328601 branch 2 times, most recently from 8a18dfb to a97e223 Compare June 13, 2022 15:20
@radeusgd
Copy link
Member

I dare anyone to review distribution/lib. I double dare you ;)

I've got a long train ride tomorrow evening, so challenge accepted ^^


## Checks equality of numbers, using an `epsilon` value.

Arguments:
- that: The number to check equality against.
- epsilon: The value by which `this` and `that` can be separated by before
- epsilon: The value by which `self` and `that` can be separated by before
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder what we should do about these?

this and that sounded nicely, now, not sure if we should change that to something else? But I'm not sure if there are good alternatives either.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is something @ekmett raised as well

Copy link
Member

@radeusgd radeusgd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've skimmed through all the changes and apart from the few comments above it looks good to me.

I'd like to discuss the that issue, but it likely can be addressed separately from this PR, as it is already huge as it is.

Copy link
Member

@JaroslavTulach JaroslavTulach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So many changed files! Let's get this done.

hubertp and others added 6 commits June 16, 2022 16:49
A semi-manual s/this/self appied to the whole standard library.
Related to https://www.pivotaltracker.com/story/show/182328601

In the compiler promoted to use constants instead of hardcoded
`this`/`self` whenever possible.
Co-authored-by: Radosław Waśko <radoslaw.wasko@enso.org>
@hubertp hubertp force-pushed the wip/hubert/this-to-self-182328601 branch from 9c65e5c to 19b4c91 Compare June 16, 2022 14:52
Copy link
Contributor

@4e6 4e6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Epic!

@hubertp hubertp added the CI: Ready to merge This PR is eligible for automatic merge label Jun 20, 2022
@hubertp hubertp requested a review from mwu-tow as a code owner June 20, 2022 09:58
@hubertp
Copy link
Contributor Author

hubertp commented Jun 20, 2022

@MichaelMauderer @mwu-tow @wdanilo @farmaazon please ignore everything apart from api/gui stuff. No need for you to waste time on this monster PR.

@mergify mergify bot merged commit 22a371a into develop Jun 21, 2022
@mergify mergify bot deleted the wip/hubert/this-to-self-182328601 branch June 21, 2022 10:53
kazcw pushed a commit that referenced this pull request Jun 29, 2022
A semi-manual s/this/self appied to the whole standard library.
Related to https://www.pivotaltracker.com/story/show/182328601

In the compiler promoted to use constants instead of hardcoded
`this`/`self` whenever possible.

# Important Notes
The PR **does not** require explicit `self` parameter declaration for methods as this part
of the design is still under consideration.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants