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

Apply class-shadowing.md to the Spec #16839

Merged
merged 1 commit into from
Feb 7, 2023

Conversation

Sporarum
Copy link
Contributor

@Sporarum Sporarum commented Feb 6, 2023

This is the first PR of porting the Scala 2 Specification (the spec) to include the changes of Scala 3, it aims to serve as an example of what a contribution to the porting of spec should look like

Atomic changes should be contained in commits called Apply <name>.md, where <name>.md is the name of a file contained in one of the subfolders of TODOreference, and the content of the commit should be:

  1. The changes to the Spec necessary to make it agree with <name>.md
    (in this case 05-classes-and-objects.md, but could be multiple files)
  2. Moving file <name>.md from a subfolder of TODOreference to the corresponding subfolder of APPLIEDreference
    (in this case class-shadowing.md)
  3. Updating <name>.md to remove all "spec-y" details, potentially deleting the whole file (e.g. if the only useful content in the file was extracted to the Spec)
  4. If the file concludes with "More info", the linked page should also go through all the previous points, and be included in the same commit
    (in this case class-shadowing-spec.md. Note that <name>-spec.md files should not be deleted just on principle !)

Commits that do not adhere to this format are allowed, if they fix something that was already applied
(In the case an important difference with Scala 2 is not present in the reference, it is better to first update the reference, and then change the Spec once that's done)

Please try to obey by the principles outlined in docs/_spec/README.md, you'll also find there how to build the spec locally.

It is preferable for PRs to contain only one commit.

Tips

Once you make a PR with a single commit, it is easier (also for the reviewer!) to incorporate feedback in temporary commits that you git squash just before merging than to constantly git ammend

@Sporarum Sporarum added the area:specification About the WIP Scala 3 Spec label Feb 6, 2023
@Sporarum Sporarum requested a review from bishabosha February 6, 2023 15:32
Copy link
Member

@bishabosha bishabosha left a comment

Choose a reason for hiding this comment

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

it looks like there is a link "More details" (leading to the spec page), that should be deleted from
docs/_spec/APPLIEDreference/dropped-features/class-shadowing.md

@bishabosha bishabosha marked this pull request as ready for review February 6, 2023 15:45
@sjrd sjrd force-pushed the class-shadowing-apply branch from 420e02d to c4a2532 Compare February 6, 2023 17:12
@sjrd
Copy link
Member

sjrd commented Feb 6, 2023

Rebased so that a single commit shows up in this PR.

@bishabosha bishabosha force-pushed the class-shadowing-apply branch from c4a2532 to 03e3bd4 Compare February 7, 2023 12:31
@bishabosha bishabosha enabled auto-merge February 7, 2023 12:39
@bishabosha bishabosha merged commit 4b0fa97 into scala:main Feb 7, 2023
sjrd added a commit that referenced this pull request May 1, 2023
This is the rest of the work I (Sporarum) did on the Spec
about feature removals.

It is not in individual PRs, but still follows the other principles
outlined in
#16839 and in
`docs/_spec/README.md`.
odersky added a commit that referenced this pull request Jun 20, 2023
…17942)

[skip ci]

Note that weak conformance was not entirely dropped. It is still
applicable to determine *compatibility* of a method call argument to the
parameter type. See #7708.

@odersky Since you asked about it in the big type system PR. ;) FYI the
changes in `TODOreference` and `APPLIEDreference` are part of [our
process to integrate the Reference into the
Spec](#16839).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:specification About the WIP Scala 3 Spec
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants