Skip to content
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

update contributing.rst and docstyle.rst: refer to a bug via bug #1234 + other guidelines #14796

Merged
merged 4 commits into from
Jun 26, 2020

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Jun 25, 2020

principle: "instead of each one guessing or following surrounding code (which encourages dialects), encourage following a single convention"

I don't mind changing that convention to something else that's reasonable, eg bug #1234 but I'd prefer to have a single fixed convention. Doesn't mean past code needs to be changed/edited but at least future code should.

it makes it easier for tooling, as well as simple grep search

refs: https://github.com/nim-lang/Nim/pull/14795/files#r445261335 which was using many different conventions in the same PR:

block: # 9313
block t9458:
# t12398
#7174

=> instead use:
block: # issue #9313

which makes it awkward to search given that 9313 can come from anywhere (eg port number etc)

note

as a side benefit, this will make those issue links clickable, with some browser extensions (eg: https://github.com/sindresorhus/refined-github) as you can see in the updated #14795; (at least when that extension works, refs refined-github/refined-github#3285)

@timotheecour timotheecour changed the title add a testing convention in nep1 add a testing convention in nep1: refer to a bug via issue #1234 Jun 25, 2020
@timotheecour timotheecour requested a review from Araq June 25, 2020 03:52
@Araq
Copy link
Member

Araq commented Jun 25, 2020

Good, but change it to "bug #123", it's shorter and I think I use it more often.

@Clyybber
Copy link
Contributor

Clyybber commented Jun 25, 2020

This doesn't belong into NEP1 IMO, but in tests/readme.md.

@alaviss
Copy link
Collaborator

alaviss commented Jun 25, 2020

Should we be really modifying NEP-1 or should we just write a new one called NEP-2? We can probably use a similar process to IETF RFC: RFCs are immutable and updates are published in the form of a new RFC.

What is the actual process for introducing these NEPs anyway? I think we kinda obsoleted this with RFCs, but I can be wrong.

@Araq
Copy link
Member

Araq commented Jun 25, 2020

Er, indeed this belongs to the contributings.rst document!

@timotheecour timotheecour force-pushed the pr_nep1_issue branch 2 times, most recently from 4d73e8f to f8aa847 Compare June 25, 2020 18:58
@timotheecour timotheecour changed the title add a testing convention in nep1: refer to a bug via issue #1234 update contributing.rst and docstyle.rst: refer to a bug via bug #1234 + other guidelines Jun 25, 2020
doc/docstyle.rst Outdated Show resolved Hide resolved
doc/docstyle.rst Outdated Show resolved Hide resolved
@timotheecour timotheecour force-pushed the pr_nep1_issue branch 2 times, most recently from 2ad8fa5 to 46a303d Compare June 25, 2020 19:10
@timotheecour
Copy link
Member Author

timotheecour commented Jun 25, 2020

Should we be really modifying NEP-1 or should we just write a new one called NEP-2?

IIUC NEP is obsoleted by RFC's repo, but nep1 became synonymous of style guide; that being said, I wouldn't mind someone (carefully) merging nep1.rst into docstyle.rst or similar, and updating any reference to nep1

Er, indeed this belongs to the contributings.rst document!

indeed.

PTAL (better debate here than in individual PR's about those guidelines)

doc/contributing.rst Outdated Show resolved Hide resolved
@Araq Araq merged commit 4dfb062 into nim-lang:devel Jun 26, 2020
@timotheecour timotheecour deleted the pr_nep1_issue branch June 26, 2020 17:11
timotheecour referenced this pull request Nov 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants