-
Notifications
You must be signed in to change notification settings - Fork 14
Overhaul documentation and add contributing guide #86
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
Conversation
blink1073
left a comment
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.
Nice, sounds good!
| 1. **Bump the version in `__init__.py` and push it to master**. | ||
| We try to use [semver version numbers](https://semver.org/) but don't stress out about it too much. | ||
| - In your commit message use something like `RLS: vX.X.X`. | ||
| - Add a changelog to `CHANGELOG.md`[^1]. |
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.
Maybe add a nox command to generate the new changelog entry, if only to print out on the console?
manics
left a comment
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.
It wasn't clear to me what prerequisite knowledge a reader should have, so some of these suggestions may not be needed.
Is there any chance you could turn on read-the-doc PR previews?
| @@ -1,42 +1,27 @@ | |||
| # github-activity | |||
|
|
|||
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.
Could be a good opportunity to add a badge?
[](https://github.com/executablebooks/github-activity/actions/workflows/tests.yaml)
|
|
||
| The easiest way to use `github-activity` to generate activity markdown is to use | ||
| the command-line interface. It takes the following form: | ||
| Use this tool via the command line like so: |
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.
It wasn't clear to me whether this is intended as a quick start in which case this should probably mention an auth token is needed, or if it's an example of how easy this is to use in which case it might be better to say After following the installation instructions you can use this tool via the command line like so: ?
docs/contribute.md
Outdated
| 2. **Draft a new release on GitHub**. | ||
| Under the [`releases` page](https://github.com/executablebooks/github-activity/releases) click [the `Draft a New Release` button](https://github.com/executablebooks/github-activity/releases/new). | ||
| - Connect the release to your release commit. | ||
| - Create a new tag for the release called `vM.m.p`. |
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 think vX.X.X as used above (or vX.Y.Z as used on https://semver.org/) is clearer than vM.m.p, it took me a while to realise it was just Major.minor.patch and not an abbreviation for something!
docs/use.md
Outdated
| you don't need to set any scopes on the token you create. | ||
| - When using `github-activity` from the command line, use the `--auth` parameter and pass | ||
| in your access token. This is easiest if you set it as an **environment variable**, | ||
| such as `MY_ACCESS_TOKEN`. You can then add it to your call like so: |
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.
From a security perspective, especially on a shared system, it's safer to set the GITHUB_ACCESS_TOKEN env-var instead of passing secrets on the command line
| These sections describe how to control the major functionality of this tool. | ||
|
|
||
| ## Generate a markdown changelog | ||
|
|
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.
Is the expectation that a user should be able to follow this from top to bottom the first time they use this tool? If so I think mentioning the need for a GitHub token first would be helpful since it's mandatory
github-activity/github_activity/github_activity.py
Lines 159 to 168 in 1ce5486
| if not auth: | |
| raise ValueError( | |
| "Either the environment variable GITHUB_ACCESS_TOKEN or the " | |
| "--auth flag or must be used to pass a Personal Access Token " | |
| "needed by the GitHub API. You can generate a token at " | |
| "https://github.com/settings/tokens/new. Note that while " | |
| "working with a public repository, you don’t need to set any " | |
| "scopes on the token you create. Alternatively, you may log-in " | |
| "via the GitHub CLI (`gh auth login`)." | |
| ) |
|
Interesting we haven't been building PRs even though it is checked on. I just cycled the check and also reopened this PR, let's see if that worked. Will respond to comments tomorrow! |
Co-authored-by: Simon Li <orpheus+devel@gmail.com>
|
OK I made a few updates to the instructions here to make it easier to follow, thanks for the comments everybody! I think this is good to go unless others would like to see more changes |
|
Sorry that pre commit keeps breaking - the install fails on my machine right now because of a word glibc error |
|
OK got the tests passing :-) I think this one is good to go from my end |
|
Just realized we never actually merged this. @minrk in the future you should feel free to just merge rather than only hitting "approve"! :-) |
This makes major updates to our documentation to add more information for contributors, and to make it easier to parse.
tests.ymlI'll get the tests passing, but wanted to get this up to confirm that @blink1073 and @minrk think this would be a useful addition to help others get started!