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

docs: Add thanks to all contributors to main README.md #2947

Merged
merged 1 commit into from
May 4, 2023

Conversation

neteler
Copy link
Member

@neteler neteler commented May 3, 2023

Add an auto-generated section listing all contributors with their icons at the bottom of README.md.

Make this README.md file less boring :-)

image

Uses:
https://contrib.rocks/preview?repo=OSGeo%2Fgrass

Add an auto-generated section listing all contributors with their icons.

Make this README.md file less boring :-)
@neteler neteler added enhancement New feature or request backport_needed manual Documentation related issues labels May 3, 2023
@neteler neteler added this to the 8.3.0 milestone May 3, 2023
@neteler neteler requested a review from veroandreo May 3, 2023 21:25
@neteler neteler self-assigned this May 3, 2023
Copy link
Contributor

@veroandreo veroandreo left a comment

Choose a reason for hiding this comment

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

beautiful ❤️

@neteler neteler merged commit 5801bf8 into OSGeo:main May 4, 2023
@neteler neteler deleted the thanks_to_contributirs branch May 4, 2023 09:56
@marisn
Copy link
Contributor

marisn commented May 18, 2023

Oh no! How is possible that this got merged?!? As I have enabled pre-commit hook (thanks @nilason ) now I can not commit anything unless I change this code too – a no-no in the middle of a merge.
Either this code needs to be changed or pre-commit linters need an exception.

$ git commit
[INFO] Checking merge-conflict files only.
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
markdownlint.............................................................Failed
- hook id: markdownlint
- exit code: 1

README.md:111 MD012/no-multiple-blanks Multiple consecutive blank lines [Expected: 1; Actual: 2]
README.md:114:2 MD033/no-inline-html Inline HTML [Element: a]
README.md:115:4 MD033/no-inline-html Inline HTML [Element: img]

black....................................................................Passed
flake8...................................................................Passed
clang-format.............................................................Passed
yamllint.................................................................Passed

neteler added a commit to neteler/grass that referenced this pull request May 18, 2023
markdown-lint: exclude README.md from 033-no-inline-html.md, i.e. allow inline HTML in README.md.

Addresses
OSGeo#2947 (comment)
@neteler
Copy link
Member Author

neteler commented May 18, 2023

Either this code needs to be changed or pre-commit linters need an exception.

Done both, see #2971

@wenzeslaus
Copy link
Member

I think --no-verify is meant for these cases like merge of non-complaint files (SE).

@wenzeslaus
Copy link
Member

... How is possible that this got merged?!? ...

...--no-verify is meant for these cases...

With that said, this should not happen. I understood that a check in CI is not possible yet because the not all files are compliant yet and doing only the "PR diff" is not readily available. Please, correct me if if I'm wrong, @nilason.

@nilason
Copy link
Contributor

nilason commented May 19, 2023

Temporarily disabling hooks: https://pre-commit.com/#temporarily-disabling-hooks

With that said, this should not happen.

To that I absolutely agree!

I understood that a check in CI is not possible yet because the not all files are compliant yet and doing only the "PR diff" is not readily available. Please, correct me if if I'm wrong, @nilason.

You're essentially right, but let me elaborate:

The main problem is that the superlinter, which checks (among others) markdown and yaml files are not in complete sync with the settings in .pre-commit-config.yaml. Furthermore, the trailing-whitespace check does not even have an equivalent, neither does the end-of-file-fixer check (the latter is however visible in GH code viewer as a stop sign).
The clang-format, black and flake8 checks are reliable however.

At the time I introduced pre-commit all files in repo were passing the checks (with the rules and exceptions in .pre-commit-config.yaml) in ordinary committing usage. It is not, however, possible to do a pre-commit run --all-files because the way pre-commit works in that mode: the .flake8 file is then not used and perhaps the black setting are also ignored.

I figured using superlinter is close enough (and considering its ease of use and with the main advantage to avoid unnecessary CI runs, everyone would use pre-commit anyway...), but close enough is obviously not good enough!

Back then I flagged for the the possible solution for this might be https://pre-commit.ci, but I haven't got to it to explore how that might be implemented.

neteler added a commit that referenced this pull request May 20, 2023
* pre-commit-config.yaml: exclude README.md from markdown-lint

markdown-lint: exclude README.md from 033-no-inline-html.md, i.e. allow inline HTML in README.md.

Addresses
#2947 (comment)

Co-authored-by: Nicklas Larsson <n_larsson@yahoo.com>
neteler added a commit that referenced this pull request May 20, 2023
* pre-commit-config.yaml: exclude README.md from markdown-lint

markdown-lint: exclude README.md from 033-no-inline-html.md, i.e. allow inline HTML in README.md.

Addresses
#2947 (comment)

Co-authored-by: Nicklas Larsson <n_larsson@yahoo.com>
@wenzeslaus wenzeslaus changed the title README.md: Thanks to all contributors docs: Add thanks to all contributors to main README.md Jun 6, 2023
neteler added a commit to nilason/grass that referenced this pull request Nov 7, 2023
Add an auto-generated section listing all contributors with their icons.

Makes this README.md file less boring :-)
neteler added a commit to nilason/grass that referenced this pull request Nov 7, 2023
)

* pre-commit-config.yaml: exclude README.md from markdown-lint

markdown-lint: exclude README.md from 033-no-inline-html.md, i.e. allow inline HTML in README.md.

Addresses
OSGeo#2947 (comment)

Co-authored-by: Nicklas Larsson <n_larsson@yahoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request manual Documentation related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants