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

Improve documentation for contributors/maintainers #4934

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mcmire
Copy link
Contributor

@mcmire mcmire commented Nov 14, 2024

Explanation

It was pointed out recently that the link to the Contributing guide is buried at the bottom of the README. This commit makes it more prominent by moving it to the top and lists the major sections of the document.

However, this commit also updates the Contributing guide to be more generally helpful and to answer questions that I've received from other teams not familiar with this repo who are unaware that codeownership and releases are shared among teams.

References

As mentioned above, this PR was inspired by a message in Slack, but also inspired by conversations I've had in the past with other teams.

Changelog

N/A

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@mcmire mcmire force-pushed the update-contributing-docs branch 5 times, most recently from 9f9925b to 6e93383 Compare November 14, 2024 21:09
It was pointed out recently that the link to the Contributing guide is
buried at the bottom of the README. This commit makes it more prominent
by moving it to the top and lists the major sections of the document.

However, this commit also updates the Contributing guide to be more
generally helpful and to answer questions that I've received from other
teams not familiar with this repo who are unaware that codeownership and
releases are shared among teams.
@mcmire mcmire marked this pull request as ready for review November 14, 2024 21:18
@mcmire mcmire requested a review from a team as a code owner November 14, 2024 21:18
@@ -2,9 +2,21 @@

This monorepo is a collection of packages used across multiple MetaMask clients (e.g. [`metamask-extension`](https://github.com/MetaMask/metamask-extension/), [`metamask-mobile`](https://github.com/MetaMask/metamask-mobile/)).

## Modules
## Contributing
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This section was previously buried at the bottom.

@@ -148,120 +173,117 @@ To use a preview build for a package within a project, you need to override the

4. Run `yarn install`.

## Adding new packages
## Releasing changes
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This section got moved up, and expanded to give people more guidance than before.

Pay attention to the box you see when you press the green button and ensure that the final name of the commit follows the pattern "Release \<new version\>".
You're done!

## Performing operations across the monorepo
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't make any changes to this section, so it should be the same.


### Handling common errors
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure the "Handling common errors" section is that useful. I've found that people reach out to me if they have issues. We can figure out a way to add it back if we need to.

Copy link
Member

@mikesposito mikesposito left a comment

Choose a reason for hiding this comment

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

Looks good!

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.

2 participants