-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
doc: improve CONTRIBUTING.md #14514
base: master
Are you sure you want to change the base?
doc: improve CONTRIBUTING.md #14514
Conversation
97a8b2c
to
a4bedbb
Compare
a4bedbb
to
04b7219
Compare
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.
one item:
.github/CONTRIBUTING.md
Outdated
[Issues Page](https://github.com/checkstyle/checkstyle/issues). We | ||
have a few issues labeled as | ||
https://github.com/checkstyle/checkstyle/labels/good%20first%20issue to | ||
help you get started. |
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.
please mention here second, third, .... We rank them on purpose to increase complexity slowly.
Such issues are for all contributors.
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.
done
04b7219
to
74279d2
Compare
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.
Ok to merge.
Thanks a lot
This comment was marked as outdated.
This comment was marked as outdated.
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.
Reviewed first document.
* [Pull Request Template](https://github.com/checkstyle/checkstyle/blob/master/.github/PULL_REQUEST_TEMPLATE.md) | ||
- Please see the [CheckStyle Documentation](https://checkstyle.org/beginning_development.html) | ||
for information on how to get started with the project. This includes setting up your | ||
development environment, building the project, and running tests. |
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 didn't see anything about running tests in the beginning development, but I think we should add it to there and not change this line.
Is this referencing the mvn install
command?
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 was referring to mvn clean verify
, but wanted to keep this general enough to allow us to change the beginning development document.
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 am going to say that this should be covered by #14542 as well. It wasn't very easy finding the commands with this text.
for information on how to get started with the project. This includes setting up your | ||
development environment, building the project, and running tests. | ||
- Take a look at the [Contribution Guidelines](https://checkstyle.org/contributing.html) for | ||
on how to contribute to the project. |
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 find it odd that beginning_development.html
has instructions on pushing, but then we go into contributing, which is really where the pushing instructions should be IMO. Then it is also odd contributing has build instructions but then we already went over beginning_development.html
which has those instructions.
Some of the commands in contributing are also not in beginning_development.html
.
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.
Yeah, this does need improvement. Let me think on how to separate and improve this.
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 can clean all this up in the scope of #14542
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.
Please make sure to mention that this page needs to be re-updated when we change the other pages.
development environment, building the project, and running tests. | ||
- Take a look at the [Contribution Guidelines](https://checkstyle.org/contributing.html) for | ||
on how to contribute to the project. | ||
- Select an issue to work on from the |
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 we should have a document for the labels. You mention first/second issue, but don't mention easy
and approved
. Both are very important to mention as new users gloss over approved on issues and jump the gun.
If we are getting rid of any old labels like easy
, then we should do that.
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.
easy
needs to go in my opinion. I have never liked this label, I remember when I started out here and struggled for a long time on an easy
issue. easy
is highly subjective to both experience with software development in general AND experience with the project.
first, second, third, etc. is good. Even something like good beginner issue
or even simple
might be ok. easy
is likely to make people feel discouraged if they are struggling with it (I know that I did).
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 to improve:
#14541
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.
@nrmancuso Were you thinking adding text for approved
will be in other issue? I was not seeing it added yet. I can understand moving the other labels to the issue as there is more to work on, but I think approved
is pretty much solidified and understood from maintainers and it definitely needs to be conveyed to contributors.
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.
yeah, good point, done
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 the new Select an issue to work on from the Issues Page
is too bloated for a bullet. Bullets should be shorter and more to the point, maybe 1-2 sentences, 3 at most. I think this clearly shows that this bullet needs to be its own section "Selecting an Issue to Contribute To" (or some form). I think the mention of the approved label should be in the first 2 sentences.
Edit: If we start this new section, "I am on it"
should probably be moved to it.
.github/CONTRIBUTING.md
Outdated
|
||
- **Read our pull request rules.** See [PR Rules](https://github.com/checkstyle/checkstyle/wiki/PR-rules). | ||
- **Comment on the issue.** If you are working on an existing issue, please comment on the issue | ||
to let others know that you are working on it ("I am on 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 think this left open ended on what if there is no existing issue.
Also this point should first. PR rules was created for already opened PRs and should probably be last.
Maybe also make a point re pointing to https://checkstyle.org/contributing.html#Submitting_your_contribution and https://checkstyle.org/beginning_development.html#Starting_Development where we talk about remotes and branches? I think some work needs to be done with the external pages some. Things are a bit all over the place.
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 this left open ended on what if there is no existing issue.
fixed
Maybe also make a point re pointing to
I would like to keep this as concise as possible and avoid repetition
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.
fixed
I am sorry, I am not seeing how this one is fixed.
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.
Additional comment.
7f420ee
to
0348f11
Compare
0348f11
to
5c637b8
Compare
https://github.com/nrmancuso/checkstyle/blob/contributing/.github/CONTRIBUTING.md I found an easier way to link to the view of the page. I am going to put this in the first post. |
Ah, cool, I didn't know about this. Nice to have a link that doesn't get outdated. |
2. Join the Checkstyle group on [Google Groups](https://groups.google.com/forum/#!forum/checkstyle). | ||
This is another place to ask questions and get to know the community. | ||
3. **Start contributing.** We have a list of issues labeled as | ||
https://github.com/checkstyle/checkstyle/labels/good%20first%20issue. These are a great place to |
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 and some other things seem like a re-hash of https://github.com/nrmancuso/checkstyle/blob/contributing/.github/CONTRIBUTING.md#rocket-getting-started and such.
Why don't we move everything from here talking about general contributions and such, and move it to CONTRIBUTING
and this page only contain extremely GSOC specific things. We should be able to link to the other MD pages like I did above.
Get to know the project shouldn't be GSOC specific. I am ok with join the community. I mentioned labels.
great for GSoC. You can find them in our wiki [Checkstyle Wiki](https://github.com/checkstyle/checkstyle/wiki). | ||
You can also propose your own project idea. We are open to new ideas and would love to hear | ||
what you are interested in working on. | ||
5. **Create a Draft Proposal.** Once you have a good understanding of the project and have an idea |
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 am not seeing anything about timing. One concern I see is new people just thinking they "joined" and will start submitting proposals. They need to do X issues / PRs first to show they have an understanding of CS and our flow. Shouldn't they then be talking to us a little bit about the projects before jumping into the proposal?
more challenging issues, like bug fixes and new features. | ||
4. **Take a Look at Our Project Ideas.** We have a list of project ideas that we think would be | ||
great for GSoC. You can find them in our wiki [Checkstyle Wiki](https://github.com/checkstyle/checkstyle/wiki). | ||
You can also propose your own project idea. We are open to new ideas and would love to hear |
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.
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 don't think this should be explicitly worded. We can discuss all and everything, but project for gsoc is very complicated process.
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.
What are we afraid of here? It’s not clear to me why we would not encourage this.
6. **Submit Your Proposal.** Now that you have refined your proposal, submit it through the | ||
GSoC website **before the final submission deadline**. | ||
|
||
## :star: Additional Tips |
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 find it odd that this tips section is right between 2 proposals pieces. Its like you are cutting your thought in half and putting something else between them. I think this should probably be moved more to the end.
Also I think the Contribute
bullet is what I recommended above for another section.
|
||
All projects aim to solve a problem or improve something. Start your proposal by introducing the | ||
problem you are trying to solve or the improvement you are trying to make. This is your opportunity | ||
to show us that you understand the problem, why it is important, and why it is worth solving. |
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.
why it is important
why it should be important to the maintainers
?
Include any relevant background information that will help us understand the problem, like | ||
statistics, examples, references to existing issues, etc. | ||
|
||
#### Project Goals |
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 isn't mentioned the time table. How long for each deliverable/milestone. There is a timeline section, so my point is somewhat pointless, but it also seems like to provide a timeline you have to resay what the deliverables/milestones are.
Also this section seems to be more questions than guidance, like you are almost saying some things are optional.
|
||
#### Implementation Plan | ||
|
||
Describe your solution to the problem. This should include a high-level overview of the |
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.
How technical should this plan be?
#### Timeline | ||
|
||
Show us that you can break down your project into manageable tasks and that you have a clear plan | ||
for how you will accomplish your goals. This should be a timeline that shows what you will be |
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 find it odd you ask for how you will accomplish your goals
but then turn around and say this is a timeline. The "HOW" should be the Implementation Plan
, this section should be only about what we will get and when.
Show us that you can break down your project into manageable tasks and that you have a clear plan | ||
for how you will accomplish your goals. This should be a timeline that shows what you will be | ||
working on each week. It should include milestones and deliverables. An effective way to present | ||
this is in a table or a Gantt chart. |
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.
For me, I am not familar with Gantt chart
by name. You think it may be beneficial to link to wikipedia or something to show an example of what you want?
We want to see that you have a clear plan. | ||
- **Be Professional.** We are looking for a proposal that is well-written and professional. | ||
- **One Proposal <-> One Project.** Do not create a proposal that spans multiple projects. | ||
- **Proposal Title** If creating a proposal based on our project ideas, please use the title of the |
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.
Why is this one a tip and not a requirement? There is nothing here that seems subjective. I consider a tip something that is beneficial but can be ignored (but probably unwise).
What are your career goals? What do you hope to get out of GSoC? What do you hope to contribute to | ||
Checkstyle? This is your opportunity to show us that you are a good fit for the project and that | ||
you are excited to work with us. **You must include a list of links to your Checkstyle contributions | ||
to be considered for GSoC; this is one of the most important selection criteria.** |
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.
Should we also mention that they should specify partial and complete contributions?
I also find it odd again with all the questions and then ending with "You must include". Maybe consider splitting this into 2 paragraphs, 1 for optional and the other for required.
|
||
[Explanation on how to create your own module](https://checkstyle.org/extending.html) | ||
- Download our [Latest Release](https://github.com/checkstyle/checkstyle/releases/) from GitHub. | ||
- Add Checkstyle to your build from [Maven Central](https://search.maven.org/search?q=g:%22com.puppycrawl.tools%22%20AND%20a:%22checkstyle%22). |
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.
https://central.sonatype.com/search?q=g:%22com.puppycrawl.tools%22%20%20a:%22checkstyle%22&smo=true
Showing 0 results out of the 594054 available packages
I am not sure I understand the purpose of this link.
[Explanation on how to create your own module](https://checkstyle.org/extending.html) | ||
- Download our [Latest Release](https://github.com/checkstyle/checkstyle/releases/) from GitHub. | ||
- Add Checkstyle to your build from [Maven Central](https://search.maven.org/search?q=g:%22com.puppycrawl.tools%22%20AND%20a:%22checkstyle%22). | ||
- Read our [Documentation](https://checkstyle.org/checks.html) for usage and configuration. |
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.
Before this item should probably be something on running. https://checkstyle.org/running.html
[Contribution Guidelines](https://github.com/checkstyle/checkstyle/blob/master/.github/CONTRIBUTING.md) | ||
for information on how to contribute to the project. This includes creating issues, submitting pull | ||
requests, and setting up your development environment. **If you are a potential Google Summer | ||
of Code (GSoC) participant, this is a great place to start.** |
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.
So we don't want to tell them about the new GSOC MD page?
|
||
[Setup IDE for development](https://checkstyle.org/beginning_development.html) | ||
## Quick Start |
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 probably a good first pass, but I feel quick start is way too quick. Usually I like to see small and quick examples and such on the README on how to use the repo. I usually go to documentation afterwards once I am looking for more complex or specific items.
working on each week. It should include milestones and deliverables. An effective way to present | ||
this is in a table or a Gantt chart. | ||
|
||
#### About You |
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 usually ask GSOC participants what days and times they are available for communication/work so we can plan collaboration days, answer questions faster, or get an idea of when progress will be made.
Improves the CONTRIBUTING.md file, README.md file, and creates the GSOC.md file for GSoC participants. Make sure to "View File" on each to see rendered markdown content.
GH Views:
https://github.com/nrmancuso/checkstyle/blob/contributing/.github/CONTRIBUTING.md
https://github.com/nrmancuso/checkstyle/blob/contributing/.github/GSOC.md
https://github.com/nrmancuso/checkstyle/blob/contributing/README.md