-
Notifications
You must be signed in to change notification settings - Fork 45
Add contributing.md with GH workflow and doc edits/creation details #8
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
|
Oh also! I want to modify the front-page README regarding this, and I'll go ahead and add a section for Experimenter engineers to set @jaredlockhart up to add docs related to the new publish status as discussed yesterday. (The latter could be done in a follow up though, I might put in a separate PR for it) |
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 need to go back through the content, but here are some initial thoughts. This is look really great BTW. I love how you've arranged the images.
sidebars.js
Outdated
| label: "Data Scientists", | ||
| items: ["data-scientists-root"], | ||
| }, | ||
| "contributing", |
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.
Another option: move "Welcome" into an expandable sidebar item, and place this Contributing page underneath it. But I'm also cool with this remaining at the bottom of the top level
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 personally like it at the top-level but at the bottom of the list so it's quite obvious, but if others prefer it to be in a category with "Welcome" I'm fine with that. @k88hudson do you have a preference?
|
|
||
| .flex { | ||
| display: flex; | ||
| } |
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 wonder how frequently we're going to see CSS changes to accommodate doc layouts 🤔 Is it worth adding a little blurb about how and where to add CSS?
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 could add a sentence in the doc in this PR about CSS changes, I'll do that with my next push.
|
Yeah so this is super cool, love the links to "do this in github" especially. Definitely worth checking if it makes sense to some folks who will be using the github UI. I think this could use some guidance around like what the process for asking for a review is / how things will get merged |
|
Thank you for the comments everyone! I've pushed some updates and believe I've addressed all comments except:
@k88hudson we talked briefly in Slack about this - how do you feel about asking users to come back to their PRs to merge them after someone r+'s? We oftentimes approve things with a comment/suggestion or two that we consider non-blocking, so that seems more ideal in case they want to make a change before merging, but if someone isn't used to hanging out in GitHub, PRs could potentially hang out longer than they need to. As @jodyheavener and I said in Slack, perhaps it's OK to require this at first and see how things go? Also, should users request the |
Over at mozilla/data-docs the convention is to explicitly tag a reviewer. I'd worry that tagging the team could be noisy. Requesting a review requires write access, so if they're not on project-nimbus they should do that first. :) "Ask in #nimbus-project" seems good, though. |
|
Re:reviewers, what about using a CODEOWNERS file to auto assign reviewers based on globs? For example, Tim+Anna are reviewers for any jetstream changes, experimenter core can be reviewed by whomever adds themselves to the codeowners file for whatever paths they volunteer to review/own. I can review PRs that add .svg files. if we add e2e testing docs, B4hand and I can be notified on changes in those paths. |
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 really great. Content is great. I'm biased but it looks extra great in dark mode. I love the image arrangement.
| Alternatively, consider subscribing to this repository in the [GitHub Slack](https://slack.github.com/) app by searching for it under "Apps" inside Slack. | ||
|
|
||
| </div> | ||
| <img src="../img/github-watch.png" alt="GitHub Watch" className="img-xs-right" /> |
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 this could be more descriptive? Like "From the repository's 'Notifications settings' menu, select 'All activity' to be notified of all changes to this repository"
|
Oh while we're here maybe you wanna update or just remove these blurbs about contributing in the main readme? https://github.com/mozilla/experimenter-docs/#using-github |
|
@pdehaan I like that idea. Let's create a follow up issue for that since it might warrant some discussion? I like Tim's suggestion of tagging someone (or the team) and/or "Ask in #nimbus-project" just for now so I can get this merged in. 😎 |
|
OK, made a few changes including:
Gonna go ahead and pull the trigger here. |
Filed as https://github.com/mozilla/experimenter-docs/issues/11 |
If we end up moving this repository back into
experimenterwe can create a follow up ticket to make any needed doc updates in this section then.Closes mozilla/experimenter#4722 / EXP-1027