-
-
Notifications
You must be signed in to change notification settings - Fork 281
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 links in Markdown files #769
Conversation
@@ -0,0 +1,16 @@ | |||
{ | |||
"ignorePatterns": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind adding ^https://github.com/asyncapi/spec/blob/.*.md$
(or similar)?
Just to avoid errors on links that are going to exist only once the particular PR is merged, like during releases of the spec. I.e. https://github.com/asyncapi/spec/runs/6190017382?check_suite_focus=true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MM I think its better to don't add it. As the action is not mandatory for merging prs, and considering this is just an exception, I would just not add this into the ignorePatterns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, it makes more sense.
@derberg I guess because of cases like this, the decision of making the "link checking workflow" required
should be taken per repo basis or we should not make it required
at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to have it useful it should be required everywhere, we just did not make it required as we know there is a bazillion of links broken and we need few weeks until we get into the stage that "we know we will not cause unexpected CI fails on PR, so we won't annoy contributors" - like we will not cause the problem that developers do not like, like fix thinks not related to PR, not caused by them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@smoya Then, I guess adding it to the ignorePatterns is the only way forward since this link will cause the weekly check and maybe the other PRs to fail after we make it mandatory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to make it required, but then yes, this should be added into the ignorePatterns, otherwise PR's for new releases might fail as links could not exist yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
/dnm |
Kudos, SonarCloud Quality Gate passed! |
Kudos, SonarCloud Quality Gate passed! |
/rtm |
* feat: merge with latest master * ci: updating spec urls to match new Information Architecture in website (#801) * docs: fix links in Markdown files (#769) Co-authored-by: Alejandra Quetzalli <alejandra.quetzalli@postman.com> Co-authored-by: Khuda Dad Nomani <32505158+KhudaDad414@users.noreply.github.com>
🎉 This PR is included in version 2.5.0-next-major-spec.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Forget about the last comment saying it was released in version 2.5.0-next-major-spec.1. I made a mistake and it created this version but it should actually be |
🎉 This PR is included in version 2.5.0-next-spec.5 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Recent comments about the release from the bot were added by mistake. More details in #899 |
Context
this PR is supposed to fix all the broken links in markdown files across the repo.