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.

Sign up for GitHub

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

Jump to bottom

Markdown linter #5494

Merged
merged 57 commits into from
May 24, 2024
Merged

Markdown linter #5494

merged 57 commits into from
May 24, 2024

Conversation

chong-he
Copy link
Member

@chong-he chong-he commented Mar 27, 2024 •
edited
Loading

Issue Addressed

Additional Info

@chong-he chong-he added the work-in-progress PR is a work-in-progress label Mar 27, 2024
AgeManning
AgeManning reviewed Mar 28, 2024
View reviewed changes
Copy link
Member

@AgeManning AgeManning left a comment

Choose a reason for hiding this comment

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

Nice!

Could we add it into the code-check test?

I recall grouping things improved the speed of CI and it seems this could fit under that test?

@chong-he
Copy link
Member Author

Nice!

Could we add it into the code-check test?

I recall grouping things improved the speed of CI and it seems this could fit under that test?

I will look into it, thanks!

@jxs
Copy link
Member

jxs commented Mar 29, 2024

can we run this conditionally, i.e. only for changes in markdown files?

@chong-he
Copy link
Member Author

can we run this conditionally, i.e. only for changes in markdown files?

Yes, the VALIDATE_MARKDOWN: true is to only run this for markdown file. Got it from: https://github.com/super-linter/super-linter?tab=readme-ov-file#configure-super-linter which states:

If you set any of the VALIDATE_[LANGUAGE] variables to true, super-linter defaults to leaving any unset variable to false (only validate those languages).

dapplion
dapplion approved these changes Apr 2, 2024
View reviewed changes
Copy link
Collaborator

@dapplion dapplion left a comment

Choose a reason for hiding this comment

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

Purely formating changes, nothing controversial. They look good! I welcome the linter

@AgeManning
Copy link
Member

Probably need to pass the actual linter first

@chong-he
Copy link
Member Author

chong-he commented Apr 3, 2024

Probably need to pass the actual linter first

Yes I am still figuring out to make it pass the CI. I will also make the linter to be in the same check with code-check later

@dapplion dapplion self-requested a review April 3, 2024 02:53
michaelsproul
michaelsproul reviewed Apr 4, 2024
View reviewed changes
book/src/validator-inclusion.md Outdated
@@ -30,17 +30,17 @@ is not the case for attestations from the _previous_ epoch.

```
Copy link
Member

Choose a reason for hiding this comment

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

try adding text here

@mergify Mergify
Copy link

mergify bot commented May 21, 2024

requeue

✅ This pull request will be re-embarked automatically

The followup queue command will be automatically executed to re-embark the pull request

@mergify Mergify
Copy link

mergify bot commented May 21, 2024 •
edited
Loading

queue

🛑 The pull request has been removed from the queue default

The queue conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

mergify bot added a commit that referenced this pull request May 21, 2024
@mergify
Merge of #5494
d3285a3
@mergify mergify bot mentioned this pull request May 21, 2024
Closed
5 tasks
@jimmygchen jimmygchen added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels May 21, 2024
@michaelsproul
Copy link
Member

@Mergifyio requeue

@mergify Mergify
Copy link

mergify bot commented May 23, 2024

requeue

✅ This pull request will be re-embarked automatically

The followup queue command will be automatically executed to re-embark the pull request

@mergify Mergify
Copy link

mergify bot commented May 23, 2024 •
edited
Loading

queue

🛑 The pull request has been removed from the queue default

The queue conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

mergify bot added a commit that referenced this pull request May 23, 2024
@mergify
Merge of #5494
752acec
@mergify mergify bot mentioned this pull request May 23, 2024
Closed
5 tasks
@michaelsproul
Copy link
Member

Ah, looks like this is failing to merge because of some formatting issues in unstable. @chong-he can you back-merge unstable to this branch, and then get CI passing again? 🙏

https://github.com/sigp/lighthouse/actions/runs/9200347323/job/25306765265

@michaelsproul michaelsproul added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-merge This PR is ready to merge. labels May 23, 2024
chong-he and others added 2 commits May 23, 2024 10:26
@chong-he
Merge pull request #7 from chong-he/unstable
3daf15e 
Merge unstable to markdown branch
@chong-he
mdlint
3f8f28a
@chong-he
Copy link
Member Author

Ah, looks like this is failing to merge because of some formatting issues in unstable. @chong-he can you back-merge unstable to this branch, and then get CI passing again? 🙏

https://github.com/sigp/lighthouse/actions/runs/9200347323/job/25306765265

Done, thanks!

@chong-he chong-he added ready-for-merge This PR is ready to merge. and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels May 23, 2024
@jimmygchen
Copy link
Member

@mergify queue

@mergify Mergify
Copy link

mergify bot commented May 23, 2024 •
edited
Loading

queue

🛑 The pull request has been removed from the queue default

The pull request conflicts with at least one pull request ahead in queue.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

@michaelsproul
Copy link
Member

@Mergifyio requeue

@mergify Mergify
Copy link

mergify bot commented May 24, 2024

requeue

❌ This pull request head commit has not been previously disembarked from queue.

@michaelsproul
Copy link
Member

@Mergifyio queue

@mergify Mergify
Copy link

mergify bot commented May 24, 2024 •
edited
Loading

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 3070cb7

@mergify mergify bot merged commit 3070cb7 into sigp:unstable May 24, 2024
29 checks passed
@chong-he chong-he mentioned this pull request May 25, 2024
Closed
@chong-he chong-he deleted the markdown-linter branch June 16, 2024 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelsproul michaelsproul michaelsproul left review comments

@AgeManning AgeManning AgeManning left review comments

@macladson macladson macladson requested changes

@dapplion dapplion Awaiting requested review from dapplion

Assignees
No one assigned
Labels
infra-ci ready-for-merge This PR is ready to merge. v5.2.0 Q2 2024
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@chong-he @jxs @AgeManning @michaelsproul @jimmygchen @dapplion @macladson