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

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

rugk
Copy link

@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
Contributor

@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 changed the base branch from master to develop October 29, 2024 19:19
@rugk
Copy link
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.

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
Copy link
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.)

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