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

Fix Sphinx toctree warnings from included CHANGELOG.md #5135

Merged
merged 40 commits into from Nov 2, 2023

Conversation

stevepiercy
Copy link
Collaborator

@stevepiercy stevepiercy commented Aug 27, 2023

See #4422

  • Prepend CHANGELOG.md with html_meta stuff for SEO.
  • Manually copy CHANGELOG.md into docs/source/release-nots/index.md. This step might be automated via release-it.
  • Use a relative symlink from inside the volto docs directory to the news directory at the volto root.
  • Only check changelog entries when running docs-linkcheckbroken in volto.
  • Find news items for docs-linkcheckbroken only in volto
  • Remove non-existent excluded files.
  • Suppress warning for news items

This PR needs to be merged before its counterpart in documentation will work: plone/documentation#1527

To test locally, checkout the branch linkcheck-news-volto in both documentation and volto.

@netlify
Copy link

netlify bot commented Aug 27, 2023

Deploy Preview for volto canceled.

Name Link
🔨 Latest commit 24e8d24
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/653b89b49491df00082aabc4

@cypress
Copy link

cypress bot commented Aug 27, 2023

Passing run #7066 ↗︎

0 561 20 0 Flakiness 0

Details:

Try more random shell commands
Project: Volto Commit: 49de905e32
Status: Passed Duration: 15:39 💡
Started: Aug 30, 2023 7:48 AM Ended: Aug 30, 2023 8:04 AM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@stevepiercy
Copy link
Collaborator Author

linkcheckbroken should have failed. I think it is not reading the files in news. I need to dig into this more later.

@stevepiercy
Copy link
Collaborator Author

Sphinx does not read the files in /news. It appears that the required symlink is never created. Would it be better to commit a symlink to the repo (probably not) or find a GitHub workflow step that will create the symlink? My GitHub Workflow DuckDuckFu has not turned up anything obvious.

@stevepiercy
Copy link
Collaborator Author

w00t! Got the symlink to work to news. Now I need to make the workflow fail when linkcheck fails.

@stevepiercy
Copy link
Collaborator Author

YAY! It fails! https://github.com/plone/volto/actions/runs/5995760347/job/16259268890?pr=5135#step:7:242

Now to clean them up so it should pass on the next run.

@stevepiercy
Copy link
Collaborator Author

Shoot, it was failing for something besides a bad link in news. I could not spot the difference between plone/documentation and volto workflows and Makefiles that would cause this. So close!

docs/source/index.md Outdated Show resolved Hide resolved
@stevepiercy
Copy link
Collaborator Author

@sneridagh @davisagli this is ready for review. Note that I modified package.json, which requires extra care.

Copy link
Sponsor Member

@davisagli davisagli left a comment

Choose a reason for hiding this comment

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

One main comment here -- I think we can simplify what is done in the "after:bump" hook, since release-it already commits changes that are made while bumping the version.

I'll leave final approval for @sneridagh, since it's important for him to be familiar with the approach here in case something goes wrong when he actually makes a release.

docs/source/index.md Show resolved Hide resolved
package.json Outdated
@@ -187,7 +187,7 @@
"yarn i18n",
"git add locales"
],
"after:bump": "pipx run towncrier build --draft --yes --version ${version} > .changelog.draft && pipx run towncrier build --yes --version ${version} && make corepackagebump VERSION=${version}",
"after:bump": "pipx run towncrier build --draft --yes --version ${version} > .changelog.draft && pipx run towncrier build --yes --version ${version} && make corepackagebump VERSION=${version} && cp CHANGELOG.md docs/source/release-notes/index.md && git add docs/source/release-notes/index.md && git commit -m 'copy CHANGELOG.md' && git push",
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I think you can leave out the git add, git commit, and git push here. release-it already pushes a commit with the changes that are made during this hook.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TIL. Should I pull it out now, or let @sneridagh fiddle around with it?

Copy link
Member

Choose a reason for hiding this comment

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

I'd move it to a makefile command (as the corepackagebump). In fact, the corepackagebump actions are similar to these ones.

Copy link
Member

@sneridagh sneridagh left a comment

Choose a reason for hiding this comment

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

@stevepiercy @davisagli please a final look before I merge.

Copy link
Collaborator Author

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for the assist!

docs/source/index.md Show resolved Hide resolved
package.json Outdated
@@ -187,7 +187,7 @@
"yarn i18n",
"git add locales"
],
"after:bump": "pipx run towncrier build --draft --yes --version ${version} > .changelog.draft && pipx run towncrier build --yes --version ${version} && make corepackagebump VERSION=${version}",
"after:bump": "pipx run towncrier build --draft --yes --version ${version} > .changelog.draft && pipx run towncrier build --yes --version ${version} && make corepackagebump VERSION=${version} && cp CHANGELOG.md docs/source/release-notes/index.md && git add docs/source/release-notes/index.md && git commit -m 'copy CHANGELOG.md' && git push",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TIL. Should I pull it out now, or let @sneridagh fiddle around with it?

Copy link
Sponsor Member

@davisagli davisagli left a comment

Choose a reason for hiding this comment

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

LGTM -- hopefully

@sneridagh sneridagh merged commit c8c0b5c into main Nov 2, 2023
51 checks passed
@sneridagh sneridagh deleted the linkcheck-news-volto branch November 2, 2023 11:52
sneridagh added a commit that referenced this pull request Nov 2, 2023
* main:
  Fix Sphinx toctree warnings from included CHANGELOG.md (#5135)
sneridagh added a commit that referenced this pull request Nov 3, 2023
* main:
  Add support for TS files in add-on registry shadowing system (#5354)
  Fix Sphinx toctree warnings from included CHANGELOG.md (#5135)
  Remove regex from sphinx-copybutton config, now that linenos are excl… (#5346)
  Release 17.3.0
  Fix DatetimeWidget on FF, the button default if no type is set is sen… (#5343)
  Call applyBlockDefaults from addBlock/insertBlock, add initialValue() configuration option for blocks (#5320)
@stevepiercy
Copy link
Collaborator Author

I forgot that there is a counterpart in Documentation that needed to be merged at plone/documentation#1527. Merged now. Let's see what happens.

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.

None yet

3 participants