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

refactor(docs): update the contributing docs #17585

Merged
merged 3 commits into from
May 30, 2023

Conversation

ckipp01
Copy link
Member

@ckipp01 ckipp01 commented May 25, 2023

This makes a bunch of small updates the contributing docs that were
recently migrated over from the scala-lang site. I think there are
actually some more updates to be done, but much of these changes are
just structural. To sort of summarize this does the following:

  • gets rid of old irrelevant things like the checklist.sh
  • removes some instructions that are no longer relevant
  • re-organizes the side bar to better group things by topic
  • gets rid of redundant information and tries to group things better
  • moves the scaladoc docs into the contributing docs

I'll add some more comments inline to further explain.

[skip community_build]

Old sidebar

Screenshot 2023-05-25 at 15 18 32

New sidebar
Screenshot 2023-05-25 at 15 18 03

Copy link
Member Author

Choose a reason for hiding this comment

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

I checked with @Kordyjan and this is no longer being used, so I just removed it since it's outdated.

Copy link
Member Author

Choose a reason for hiding this comment

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

The information that was in here is now included in the other-debuggin.md page.

@@ -43,7 +50,6 @@ Start by cloning the repository:
```bash
$ git clone https://github.com/lampepfl/dotty.git
$ cd dotty
$ sbt managedSources # Needed for IDE import to succeed
Copy link
Member Author

Choose a reason for hiding this comment

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

I've never used this and testing with Metals, this doesn't seem to be necessary.

Copy link
Member

Choose a reason for hiding this comment

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

Excellent! Reading the current version, I was just going to say we should remove this!

docs/_docs/contributing/index.md Show resolved Hide resolved
Copy link
Member Author

Choose a reason for hiding this comment

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

Another one that I'll want your thoughts on @anatoliykmetyuk. This is a cool example, but also very Sublime specific. I'm not sure how helpful or relevant this really is to people contributing. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

I really didn't think this added anything to the guide. It's useful but also captures in other various places.

Copy link
Member Author

Choose a reason for hiding this comment

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

There was multiple places where there was IDE related stuff to I put them all together in the setting-up-your-ide.md.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wasn't really sure why this was here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Same with this one, I didn't see why this was part of this guide.

Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of having some stuff in readmes and others in the guide I moved all this information to a scaladoc section of the contributing guide.

Copy link
Contributor

@julienrf julienrf left a comment

Choose a reason for hiding this comment

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

Overall, I think this is a great improvement, thank you! I am a bit worried about all the URL changes, though. I know we don’t promise stable URLs in this website but I am not very comfortable with breaking many of the previous URLs. Would you mind adding a redirectFrom directive in the frontmatter of the changed pages to keep the old URLs working?

@ckipp01
Copy link
Member Author

ckipp01 commented May 25, 2023

Overall, I think this is a great improvement, thank you! I am a bit worried about all the URL changes, though. I know we don’t promise stable URLs in this website but I am not very comfortable with breaking many of the previous URLs. Would you mind adding a redirectFrom directive in the frontmatter of the changed pages to keep the old URLs working?

Do we need to worry about that here though? Since this is actually never part of the stable reference it does give us a bit of flexibility here to really just ensure that the contributing link works, and from there, who knows. I can totally do it, just want to be sure it's worth it.

@som-snytt
Copy link
Contributor

As someone who has bookmarked these unstable URLs in the past and was annoyed to update them, I think it is not worth the added effort on your part here and also forever in the future.

Also, thanks, I've already benefited from reading this PR!

@Dedelweiss
Copy link
Contributor

I see a cheatsheet.md page but I don't see it in any part.

@Dedelweiss
Copy link
Contributor

Strange, the list bugs for cause.md on my generated document but not on the preview.
Screenshot 2023-05-25 at 17 26 21

Screenshot 2023-05-25 at 17 27 57

@ckipp01
Copy link
Member Author

ckipp01 commented May 26, 2023

Strange, the list bugs for cause.md on my generated document but not on the preview. Screenshot 2023-05-25 at 17 26 21

Screenshot 2023-05-25 at 17 27 57

This is super odd, I have no idea where the 4 comes from there. I'll try to minimize because maybe this is just a general bug that we're hitting on with scaladoc.

This makes a bunch of small updates the contributing docs that were
recently migrated over from the scala-lang site. I think there are
actually some more updates to be done, but much of these changes are
just structural. To sort of summarize this does the following:

- gets rid of old irrelevant things like the `checklist.sh`
- removes some instructions that are no longer relevant
- re-organizes the side bar to better group things by topic
- gets rid of redundant information and tries to group things better
- moves the scaladoc docs into the contributing docs

I'll add some more comments inline to further explain.

[skip community_build]
Copy link
Contributor

@julienrf julienrf left a comment

Choose a reason for hiding this comment

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

Thank you!

@ckipp01 ckipp01 merged commit 28915c4 into scala:main May 30, 2023
@ckipp01 ckipp01 deleted the contributingDocs branch May 30, 2023 10:59
@mbovel
Copy link
Member

mbovel commented May 30, 2023

It's really great that this PR introduced a ”Setup your IDE” page as this is very central to getting started on Dotty.

I think the "build server" part for VS Code would deserve to be detailed/emphasized/pushed to the front even more. As an example, here are the instructions I sent to newcomers to the Spree today: #17211 (comment). These seem to be the most important steps to me.

@ckipp01
Copy link
Member Author

ckipp01 commented May 30, 2023

It's really great that this PR introduced a ”Setup your IDE” page as this is very central to getting started on Dotty.

I think the "build server" part for VS Code would deserve to be detailed/emphasized/pushed to the front even more. As an example, here are the instructions I sent to newcomers to the Spree today: #17211 (comment). These seem to be the most important steps to me.

Ah sure, I can expand that section.

ckipp01 added a commit to ckipp01/dotty that referenced this pull request May 30, 2023
This is a follow-up from the comment
[here](scala#17585 (comment))
and just adds a bit more details about making sure you import the
project correctly.
ckipp01 added a commit to ckipp01/dotty that referenced this pull request May 30, 2023
This is a follow-up from the comment
[here](scala#17585 (comment))
and just adds a bit more details about making sure you import the
project correctly.

[skip community_build]
ckipp01 added a commit to ckipp01/dotty that referenced this pull request May 30, 2023
This is a follow-up from the comment
[here](scala#17585 (comment))
and just adds a bit more details about making sure you import the
project correctly.

[skip community_build]
ckipp01 added a commit that referenced this pull request Jun 2, 2023
#17628)

This is a follow-up from the comment

[here](#17585 (comment))
and just adds a bit more details about making sure you import the
project correctly.

[skip community_build]
Kordyjan pushed a commit that referenced this pull request Nov 17, 2023
This makes a bunch of small updates the contributing docs that were
recently migrated over from the scala-lang site. I think there are
actually some more updates to be done, but much of these changes are
just structural. To sort of summarize this does the following:

- gets rid of old irrelevant things like the `checklist.sh`
- removes some instructions that are no longer relevant
- re-organizes the side bar to better group things by topic
- gets rid of redundant information and tries to group things better
- moves the scaladoc docs into the contributing docs

I'll add some more comments inline to further explain.

[skip community_build]

_Old sidebar_

<img width="321" alt="Screenshot 2023-05-25 at 15 18 32"
src="https://github.com/lampepfl/dotty/assets/13974112/2e493110-8098-445c-b101-166998c7c95f">

_New sidebar_
<img width="321" alt="Screenshot 2023-05-25 at 15 18 03"
src="https://github.com/lampepfl/dotty/assets/13974112/2f86764e-bc3e-4356-9f56-006197a10153">
[Cherry-picked 28915c4]
Kordyjan pushed a commit that referenced this pull request Nov 17, 2023
#17628)

This is a follow-up from the comment

[here](#17585 (comment))
and just adds a bit more details about making sure you import the
project correctly.

[skip community_build]
[Cherry-picked b2aaec8]
Kordyjan added a commit that referenced this pull request Nov 20, 2023
Backports #17585 to the LTS branch.

PR submitted by the release tooling.
@Kordyjan Kordyjan added this to the 3.3.2 milestone Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants