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 contribution guidelines, remove redundant files #1181

Merged
merged 12 commits into from
Aug 29, 2023

Conversation

the-right-joyce
Copy link
Contributor

No description provided.

@the-right-joyce the-right-joyce marked this pull request as ready for review August 25, 2023 19:34
@the-right-joyce the-right-joyce added the T11-documentation This PR/Issue is related to documentation. label Aug 26, 2023
docs/CONTRIBUTING.md Outdated Show resolved Hide resolved
Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not check all links but we can do another cleanup sweep through the while project afterwards

docs/STYLE_GUIDE.md Outdated Show resolved Hide resolved
docs/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
docs/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
@davxy
Copy link
Member

davxy commented Aug 28, 2023

@the-right-joyce nitpick.

What about formatting these text files following some terminal friendly convention?

A lot of devs open these files in their terminal a not markdown-aware text editor (i.e. which doesn't render markdown) and IMHO is a good idea to follow some widespread de facto formatting conventions for documentation text files.

Like:

  • give a max to line lengths (100 is a common value)
  • leave an empty line between markdown paragraphs/chapters tags (before and after)

All this stuff is not important if you read/write markdowns on github. But can be annoying on the terminal


Some references:

@davxy
Copy link
Member

davxy commented Aug 29, 2023

@the-right-joyce The first article says that the right choice is be between 50–75,
Well IMO that is too short as well. Again IMO the right value nowadays is ~100.

Again this is not super relevant, and if you can't automate it using the right tool then I can give these files a pass after merge.

@the-right-joyce
Copy link
Contributor Author

@Rashmirreddy please check the documentation guideline copy and see if you'd like to leave the format as is or take the advised format from @davxy
If not, I will just delete it :)

@chevdor
Copy link
Contributor

chevdor commented Aug 29, 2023

The idea to have properly formatted markdown is good but that should be done with clear rules and a linter such as markdownlint able to check them, not a list of text rules burried in some text somewhere.

Defining a linter's rules is usually a topic that requires consensus and sounds out of the scope of this PR. Since Markdown is also used by rustdoc, we need to carefully pick rules that will result in proper rustdoc as well.

I would suggest one pass of the standard rules of markdownlint to bring everything to a coherent state (I can commit to this PR if wanted). We can then address the linting topics in another PR and discuss the config of the linter.
NOTE: a pass of markdownlint will not address the line length issues automatically. Seeing the current values, a sane start would be to limit to a big value such as 120 to avoid insanely long lines. This can then be further reduced in the linting PR.

By the way, why are all the files all caps ?
This is usually done for a few important files at the root such as contrib or license, but we don't need to use CAPS for all markdown files.

@the-right-joyce
Copy link
Contributor Author

By the way, why are all the files all caps ?

It was copied from the repos, just moved to one single folder to not have 3x the same docs

@chevdor
Copy link
Contributor

chevdor commented Aug 29, 2023

It was copied from the repos, just moved to one single folder to not have 3x the same docs

ok, I understand this PR is more about bringing those files together so let's ignore the all CAPS for now if you want. We can address that in the linting PR.

@chevdor chevdor mentioned this pull request Aug 29, 2023
3 tasks
@chevdor
Copy link
Contributor

chevdor commented Aug 29, 2023

I created a new issue for the linting topics:
#1243

@Rashmirreddy
Copy link
Contributor

@the-right-joyce - We can go ahead with the format you have suggested. We can make changes as we progress.

@ggwpez ggwpez enabled auto-merge (squash) August 29, 2023 13:15
@ggwpez ggwpez merged commit 7c69d14 into master Aug 29, 2023
76 of 99 checks passed
@ggwpez ggwpez deleted the the-right-joyce-update-guidelines branch August 29, 2023 13:47
@chevdor chevdor mentioned this pull request Aug 30, 2023
2 tasks
Daanvdplas pushed a commit that referenced this pull request Sep 11, 2023
* update contribution guidelines, remove redundant files

* removing doc ref labels, updating links on contribution

* add manifest formatting

* update title

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* update links to the new repo

* terminal friendly convention

* update doc guideline format

---------

Co-authored-by: Alexander Samusev <41779041+alvicsam@users.noreply.github.com>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
* update contribution guidelines, remove redundant files

* removing doc ref labels, updating links on contribution

* add manifest formatting

* update title

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* update links to the new repo

* terminal friendly convention

* update doc guideline format

---------

Co-authored-by: Alexander Samusev <41779041+alvicsam@users.noreply.github.com>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T11-documentation This PR/Issue is related to documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants