Rework CONTRIBUTING file

[tomschr]

This PR fixes #342 and contains:

• All prior content was moved to RELEASE.rst.
• In CONTRIBUTING.rst:
  • Encourage people to contribute 😉
  • Address target group
  • Use CONTRIBUTING.rst for contributors only
  • Provide step-by-step instructions which can be used as a checklist

---

This is just a draft. It's an example how such text could look like. Feel free to suggest improvements.

Some aspects are a bit more verbose; probably advanced developers don't need that. However, not every developer is the same, nor does everybody know about the Git workflow. 😉

It's up to you to decide if this is enough or it should be shortened.

[codecov]

Codecov Report

Merging #343 (102626c) into master (9747174) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #343   +/-   ##
=======================================
  Coverage   97.85%   97.85%           
=======================================
  Files          22       22           
  Lines        1399     1399           
  Branches      130      130           
=======================================
  Hits         1369     1369           
  Misses         15       15           
  Partials       15       15           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9747174...102626c. Read the comment docs.

[adiroiban]

Looks good. Only minor changes.

I think the main issue with the current version is that it talks about stdlib unittest and we use twisted.trial

In terms of contributing, maybe as part of this PR we can also add a PR template in which we add a quick checklist for new contributors

• Make sure changes are covered by existing or new tests
• Make sure local test run is green for at least one python version
• Create a file in src/towncrier/newsfragments/
• Make sure all CI tests are green (this is checking all of the above)

---

My comments about tox are optional.

I think that the current PR is already an improvement over what we have in trunk so it can be merged.

But please consider mentioning that we use trial and maybe also run not only flake8 but also tox -e flake8,check-manifest,check-newsfragment

Thanks!

[tomschr]

I think the main issue with the current version is that it talks about stdlib unittest and we use twisted.trial [...]
But please consider mentioning that we use trial and maybe also run not only flake8 but also tox -e flake8,check-manifest,check-newsfragment

Sure! I will change that.

Regarding tox, I was thinking about opening another issue about tox itself. IMHO, the tox.ini could be improved to add a description key for each target. This allows tox to be called as tox -av and outputs a nice summary of all the targets. But it's only tangentially related to this issue. 😉

In terms of contributing, maybe as part of this PR we can also add a PR template in which we add a quick checklist for new contributors

Great idea! 👍 I will add that too.

I think that the current PR is already an improvement over what we have in trunk so it can be merged.

Thanks. 😊 Let me correct the your above issues first. 😉

[tomschr]

In terms of contributing, maybe as part of this PR we can also add a PR template

I've created a first draft in commit 0de1289. Not sure if this is enough. Probably something is missing.

Let me know what you think about.

[adiroiban]

Thanks. Looks good...

But as mentioned over IRC there are broken links

I am using this page to view the rendered version https://github.com/tomschr/towncrier/blob/feature/342-contributing/CONTRIBUTING.rst

[tomschr]

[...] there are broken links

You mean probably the lines which contain :ref:? Well, I've recognized that too. However, it's a valid syntax construct in RST/Sphinx. Unfortunately, it isn't rendered correctly in GitHub.

For the time being, I could replace them. However, if you plan a documentation which goes beyond little READMEs, I would highly recommend to use them.

For example, if you think a short user guide could be helpful, we could host the HTML version on ReadTheDocs and link to the rendered version. This would avoid these "broken" links. 😉

[adiroiban]

The main broken link is

Open a new topic on our GitHub discussion page.

But I think that is also best to don't have RST references so that the page looks ok with GitHub rendering.

Thanks

[tomschr]

The main broken link is

Open a new topic on our GitHub discussion page.

But I think that is also best to don't have RST references so that the page looks ok with GitHub rendering.

Not sure I understand you correctly. Maybe your definition of "broken" is different than mine. 😉 Can you explain why do see the link as "broken"?
It's the same with the issue tracker, only the texts are different. Both are rendered just fine in GitHub: They are texts that are linked to a URL.

The main idea was most of our users will read this text online. They just have to click the text to get redirected. IMHO it's not so important to actually see the link, it's more important that the link is correct.

Or do you prefer the link shown literally? Like in this step:

Fork our project on GitHub using this link: https://github.com/twisted/towncrier/fork

It's possible, but it makes the text longer.

[tomschr]

But I think that is also best to don't have RST references so that the page looks ok with GitHub rendering.

I've updated the file now, find the latest rendered output in my fork. 🙂

Better?

If yes, I can squash the commits into some logical units.

[adiroiban]

Sorry for being brief.
I tried to comment inline on broken links.

Maybe also add info about the Freenode #twisted-dev IRC channel. Some towncrier devs hang out there.

Thanks!

[adiroiban]

If yes, I can squash the commits into some logical units.

Thanks. Yes. You can do that before a merge.

[altendky]

I didn't dig into the details on this but it does seem like you can have internal links in rst on GitHub.

https://gist.github.com/ionelmc/e876b73e2001acd2140f#211inline-markup

I'll note that several 'regular' links were also broken for me. Both broken in terms of linking to the incorrect place (not valid pages) and in terms of rendering. Anyways, I'll take a look again after the above comments are handled.

[tomschr]

Maybe also add info about the Freenode #twisted-dev IRC channel.

Good idea! 👍 I've included it in the file now. The other issues are addres

squash the commits into some logical units.

Done. 🙂

@altendky Kyle, that would be great if you could have a look too! 👍

broken in terms of linking to the incorrect place

Hmn, I've checked them and all work for me. 🤔

[altendky]

@tomschr, thanks for all the work here. I know it takes awhile to write up documentation like this, hence us not having it... until now. :] I've got a few more suggestions and corrections. I don't like to use other people as my keyboard, but I also don't like to just take over their PRs. I'm game for addressing my own concerns here if you prefer.

Thanks for your help here and elsewhere recently.

[tomschr]

Thanks @altendky, your feedback is very appreciated! 👍

@tomschr, thanks for all the work here. I know it takes awhile to write up documentation like this, hence us not having it... until now. :]

Happy to help here. Actually I saw, well, the need when I first made my contribution. 😉

I've got a few more suggestions and corrections. I don't like to use other people as my keyboard, but I also don't like to just take over their PRs. I'm game for addressing my own concerns here if you prefer.

That's perfectly fine! 👍 Thank you and don't worry. I expected that this PR creates some discussion and work. That is natural when you write documentation. It's my daily job, so writing is rewriting. 😉

You found some important issues which I'm very happy you addressed. I already included it. There is only one item in your list that I would like to discuss with you. Maybe I misunderstood your idea.

Thanks for your help here and elsewhere recently.

No problem. 🙂

[tomschr]

I've squashed it and added both of you as co-authors.

After looking over it again, the "Ways to communicate and contribute" section could be split into two separate sections: "Ways to communicate" and "Ways to contribute".

The idea is to put into the first section the GitHub discussion and the item about IRC. The section about contribution contains the rest.

Would that make sense?

[altendky]

I'm fine with it split or not. Whatever you prefer. I think that I only noticed we had discussions when I was reading through this. I know some people are adamant that discussions can't happen in issues... I'm not so strict and don't know how much value discussions bring vs. issues with a 'question' tag or whatever. But, that's a separate topic from this PR.

[tomschr]

Thanks @altendky for your answer. 👍 I'm fine to leave it as is. If you like the current state, you can merge it. Let me know if you have further changes.

Thanks!

[altendky]

@adiroiban, just checking back to see if your requested change was addressed.

[adiroiban]

So.... after #368 is merged (since #89) is stalled we can integrate this docs into the main doc.

[adiroiban]

Let's merge this as this is already a big step.

I did a small update to include info about pre-commit.

I hope we can include check-manifest and check-towncrier as part of the standard pre-commit framework