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: fix broken link for doc for great_expectations.md #1664

Merged
merged 2 commits into from
Mar 5, 2025

Conversation

rugk
Copy link
Contributor

@rugk rugk commented Oct 21, 2024

I know it's deprecated (which is also a little bit too subtle to be documented IMHO), but the link should work, at least.

@rugk rugk changed the title Fix broken link for doc for great_expectations.md docs: fix broken link for doc for great_expectations.md Oct 22, 2024
@fabclmnt fabclmnt self-requested a review October 28, 2024 17:43
Copy link
Collaborator

@fabclmnt fabclmnt left a comment

Choose a reason for hiding this comment

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

Hi @rugk ,

thank you for your contribution. Nevertheless your commit messages do not comply with the expected format (eg feat: (description) or fix: (description)).

Please to contribute to our package make sure that you comply with that naming and convention. For now I've forced the change.

@fabclmnt fabclmnt force-pushed the patch-2 branch 2 times, most recently from 95a5a83 to 915f7d6 Compare October 29, 2024 19:19
@fabclmnt fabclmnt changed the base branch from master to develop October 29, 2024 19:19
@rugk
Copy link
Contributor Author

rugk commented Oct 31, 2024

Ah I thought the format you've followed was https://www.conventionalcommits.org/, which is why I instinctively used docs: (though it would also be hard to say whether this is a fix, yeah probably).

Unfortunately, that requirement is also not written on https://github.com/ydataai/ydata-profiling/blob/develop/CONTRIBUTING.md only can only see it when it's too late aka when a PR/commit is made… (in the CI or so)

Okay the CI still says:

Error: You have commit messages with errors

But not which one?
And I see you used docs actually, so I was on the right track (for a different PR maybe, dunno what commit message I've used here), and the CI requires this(?), but it does not seem to work?

And thanks for fixing BTW, AFAIK you can also squash the commit when merging and get to enter a different commit message then.

@rugk
Copy link
Contributor Author

rugk commented Oct 31, 2024

Okay maybe it was the line length… anyway, it's a bit frustrating for me to guess what the CI actually wants from me… (Edit: ah and now the CI does not run because of that GitHub workflow approval ha, oh well… I hope it is fine for you now.)

fabclmnt and others added 2 commits March 4, 2025 15:56
I know it's deprecated (which is also a little bit too
subtle to be documented IMHO), but the link should work, at least.
@fabclmnt
Copy link
Collaborator

fabclmnt commented Mar 5, 2025

Okay maybe it was the line length… anyway, it's a bit frustrating for me to guess what the CI actually wants from me… (Edit: ah and now the CI does not run because of that GitHub workflow approval ha, oh well… I hope it is fine for you now.)

Based on what I see from the error from the linter, the error message indicates that your subject line starts with a capital letter, which is considered "sentence-case" (like a normal sentence) or "pascal-case" (where each word starts with a capital letter), which is not allowed by the linter's configuration.

Message from the linter error.

input: docs: Fix broken link for doc for great_expectations.md

I know it's deprecated (which is also a little bit too
subtle to be documented IMHO), but the link should work, at least.
✖   subject must not be sentence-case, start-case, pascal-case, upper-case [subject-case]

Ideally the history of commits needs to be updated in order to remove any upper case.

Thank you for your contribution !! :)

@fabclmnt fabclmnt merged commit 8910e19 into ydataai:develop Mar 5, 2025
10 of 11 checks passed
@rugk rugk deleted the patch-2 branch March 6, 2025 19:33
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.

2 participants