Skip to content

Work around spelling exceptions added to release-next#591

Merged
jetstack-bot merged 1 commit intocert-manager:masterfrom
maelvls:work-around-spelling-on-branches
Jun 9, 2021
Merged

Work around spelling exceptions added to release-next#591
jetstack-bot merged 1 commit intocert-manager:masterfrom
maelvls:work-around-spelling-on-branches

Conversation

@maelvls
Copy link
Member

@maelvls maelvls commented Jun 9, 2021

The problem

Whenever we run mdspell, we first clone every branch (release-1.3, release-1.4, release-next...) into the content/en/docs/ folder.

When words are added to the release-next branch and aren't on master yet, master fails due to the yet-to-be-ignored words are unknown to mdspell.

As an example, the PR #570 added the new spellings "andreas-p" and "renewBefore" to the release-next branch. After the merge, all builds on master started failing since "andreas-p" and "renewBefore" were not present on the master branch's .spelling file.

Idea 1 (this PR)

Ignore spelling of release-next. PRs merged to release-next already have no spelling mistake, so we don't really need to enforce the spelling again.

!content/en/next-docs/**/*.md

Idea 2

Use mdspell's --target-relative which requires copying the .spelling file from the release-next branch and copy it into content/en/next-docs. After looking at verify-lint, one way could be to have a new flag to hugo-multiversion, e.g.,

 ./scripts/bin/multiversion" \
 	--repo-url https://github.com/cert-manager/website.git \
 	--repo-content-dir content/en/docs \
+ 	--repo-content-extra-file .spelling \
 	--output-dir "${REPO_ROOT}/content/en" \
 	--branches v0.13-docs=release-0.13 \
 	--branches v0.12-docs=release-0.12 \
 	--branches next-docs=release-next

Not sure what is the right solution 😅

cc @irbekrm @munnerz

@jetstack-bot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@jetstack-bot jetstack-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 9, 2021
@maelvls maelvls force-pushed the work-around-spelling-on-branches branch from ec648bb to cf89afa Compare June 9, 2021 07:13
@maelvls maelvls requested a review from munnerz June 9, 2021 07:31
@maelvls maelvls marked this pull request as ready for review June 9, 2021 09:52
@jetstack-bot jetstack-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 9, 2021
Copy link
Contributor

@irbekrm irbekrm left a comment

Choose a reason for hiding this comment

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

Looks good to me and makes sense- I have added a tiny nit that you can also ignore 😅

@jetstack-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: irbekrm, maelvls

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@irbekrm
Copy link
Contributor

irbekrm commented Jun 9, 2021

Looks good to me, I've added a small nit, but do unhold if you'd rather merge.

/hold
/lgtm

@jetstack-bot jetstack-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 9, 2021
@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 9, 2021
Signed-off-by: Maël Valais <mael@vls.dev>
@maelvls maelvls force-pushed the work-around-spelling-on-branches branch from cf89afa to 09f3762 Compare June 9, 2021 11:06
@jetstack-bot jetstack-bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 9, 2021
@irbekrm
Copy link
Contributor

irbekrm commented Jun 9, 2021

Looks good to me! Thanks for fixing this.

/hold cancel
/lgtm

@jetstack-bot jetstack-bot added lgtm Indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jun 9, 2021
@jetstack-bot jetstack-bot merged commit 8f70748 into cert-manager:master Jun 9, 2021
@jetstack-bot jetstack-bot mentioned this pull request Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants