Skip to content

Update CONTRIBUTING.md #2477

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

Merged
merged 5 commits into from
Jun 12, 2024
Merged

Update CONTRIBUTING.md #2477

merged 5 commits into from
Jun 12, 2024

Conversation

parispittman
Copy link
Member

What do you think about an updated file with an added a header with language from the /swift repo?

Also added some headers for pull requests and a paragraph about the repo + highlighting the forums as the main place for conversation.

None of the process language was changed. Mostly reformatting to style.

What do you think about an updated file with an added a header with language from https://github.com/apple/swift?tab=readme-ov-file#contributing-to-swift ?

Also added some headers for pull requests and a paragraph about the repo + highlighting the forums as the main place for conversation.
CONTRIBUTING.md Outdated
Comment on lines 16 to 19
Substantive changes to:
- existing proposals should only be made with the permission of the proposal authors. Substantive changes to a proposal in the Accepted or Rejected states are discouraged and require the approval of the appropriate evolution workgroup. Substantive changes to a proposal in the Active Review state require the approval of the appropriate evolution workgroup and should be advertised in the review thread.
- change vision documents require the approval of the appropriate evolution workgroup.
- documented evolution process require the approval of the Core Team.
Copy link
Member

Choose a reason for hiding this comment

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

I found this hard to read when the new text separates the verb from its object across bullet points. It forces you to be inconsistent in the first bullet, since there's more than one sentence.

Copy link
Member Author

Choose a reason for hiding this comment

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

@amartini51 agreed after re-reading it with your comment in mind. I added the verb back. What do you think? thanks for you review

Copy link
Member Author

Choose a reason for hiding this comment

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

@amartini51, @rjmccall had another suggestion below that I merged in. Let me know your thoughts for the latest copy.

Copy link
Contributor

@rjmccall rjmccall left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@xedin xedin 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!

@parispittman parispittman merged commit cd8dacd into main Jun 12, 2024
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.

4 participants