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

*: enable markdown_marker checker #16708

Merged
merged 1 commit into from
Oct 13, 2023
Merged

*: enable markdown_marker checker #16708

merged 1 commit into from
Oct 13, 2023

Conversation

fuweid
Copy link
Member

@fuweid fuweid commented Oct 8, 2023 •
edited
Loading

Part of #15549

It's trying to re-enable the markdown_marker checker in the repo because we have the old issue #7123.

However, the marker tool doesn't support the github-like relative path render.
The /absolute-path is useful and easier to be maintained in the github repo than ../../.

This is [MAINTAINER](/MAINTAINER) file.

Filed to crawford/marker#23 and hope @crawford can give some inputs.

CI Result: https://github.com/etcd-io/etcd/actions/runs/6446242086/job/17501156794?pr=16708#step:9:459

'markdown_marker' started at Sun Oct  8 07:23:06 UTC 2023
% 'marker' '--skip-http' '--root' './Documentation'
'markdown_marker' PASSED and completed at Sun Oct  8 07:23:06 UTC 2023
SUCCESS

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

@fuweid fuweid mentioned this pull request Oct 8, 2023
Closed
7 tasks
@fuweid fuweid force-pushed the enable-marker-check branch 2 times, most recently from d3c4571 to 99a5492 Compare October 11, 2023 14:55
@fuweid
Copy link
Member Author

fuweid commented Oct 13, 2023

Updated: @crawford has accepted that patch. Waiting for new release of marker and then mark this as reviewable.

@crawford
Copy link
Contributor

Marker v0.9.0 is fresh out of the oven. Thanks again for your contribution, @fuweid.

@fuweid fuweid marked this pull request as ready for review October 13, 2023 06:07
@fuweid
Copy link
Member Author

fuweid commented Oct 13, 2023

@crawford thank you! Verified it and it's working.

REF: https://github.com/etcd-io/etcd/actions/runs/6504599480/job/17666787379?pr=16708

PASSES="markdown_marker" ./scripts/test.sh
Running with --race
Starting at: Fri Oct 13 06:04:06 UTC 2023

'markdown_marker' started at Fri Oct 13 06:04:06 UTC 2023
% 'marker' '--skip-http' '--allow-absolute-paths' '--root' '/home/runner/work/etcd/etcd' '-e' './CHANGELOG' '-e' './etcdctl' '-e' 'etcdutl' '-e' './tools'
'markdown_marker' PASSED and completed at Fri Oct 13 06:04:06 UTC 2023
SUCCESS

ahrtr
ahrtr approved these changes Oct 13, 2023
View reviewed changes
Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

LGTM with a comment/question #16708 (comment)

@serathius
Copy link
Member

One problem this PR surfaces is the increased number of validators that are not available locally. Could we consider adding more installation script in the Makefile? make install-markdown-maker

@fuweid
Copy link
Member Author

fuweid commented Oct 13, 2023

One problem this PR surfaces is the increased number of validators that are not available locally. Could we consider adding more installation script in the Makefile? make install-markdown-maker

Yes. Sorry I didn't mention it. I do have plan to handle this in the follow-up.
Currently, we have https://github.com/etcd-io/etcd/blob/main/scripts/install-marker.sh to download the blob.
I am still thinking how to make the download easier.

@serathius
Copy link
Member

Thanks for already thinking about this. Let's not block on this then.

@serathius serathius merged commit d0d0c33 into etcd-io:main Oct 13, 2023
27 checks passed
@fuweid fuweid deleted the enable-marker-check branch October 13, 2023 09:59
@fuweid fuweid mentioned this pull request May 27, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@serathius serathius serathius approved these changes

@ahrtr ahrtr ahrtr approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fuweid @crawford @serathius @ahrtr