-
Notifications
You must be signed in to change notification settings - Fork 27
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
docs: Revamp docs to highlight use cases and explain EMU attribution flow #332
Conversation
Fixes #115 Update the README to highlight the problem statement and show how the app works from a user perspective. Add a new `docs/developing.md` file. - Include detailed information on developing the app, previously in the README. - Provide instructions for prerequisites, getting started, GitHub App configuration, running the app with Docker, testing, linting, building, and deployment. Add a new `docs/attribution-flow.md` file. - Describe how attributions work and how to configure git-config for proper attribution. - Provide an example of the attribution flow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great addition! I left two ideas on the issue that are non-blocking. I'd also be interested in review from some of our other maintainers
So, to ensure that contributions made by an EMU are properly attributed to their public GitHub account, the user needs to configure their local git-config to use an email address associated with their public account. This should be done at the repository level, when they are working in a private mirror managed by PMA, rather than as a global configuration. | ||
|
||
```sh | ||
git config --local user.email "[email protected]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some advanced git config examples that can make this automatic based on the remote or directory in which the repository exists. This also makes it easier to configure things like commit signing keys.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's awesome, happy to iterate on this - I'd definitely say that as I'm talking to new people about PMA, the attribution question (and more generally, how things work across the EMU<>Public boundary) is the number one concern. So having more detailed guidance here can only help!
|
||
## Prerequisites | ||
|
||
- Node.js (version 18 or higher) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may want to list a single or couple of supported versions (likely Node.js 20 and/or 22). We likely do not want to support non-LTS Node.js versions, and our dependencies also may limit the Node.js versions they support.
I'm happy to split an issue off to discuss this further. It would include
- Specify in
engines
(anddevEngines
, which is a newer feature but not sure if it is on the Node.js included version or npm yet) the versions we support. - Configure GitHub Actions to use these specific versions of Node.js (right now they use the defaults from the runners and no node setup at all, from what I can tell). Using the setup actions could also help with dependency caching.
- Add a
.nvmrc
or.node-version
file and document how developers of the app can use open source tools to install the proper Node.js version for the app.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue #333 has been created to continue this discussion!
Fixes #115
Update the README to highlight the problem statement and show how the app works from a user perspective.
Add a new
docs/developing.md
file.Add a new
docs/attribution-flow.md
file.