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

Add initial Towncrier integration #469

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

webknjaz
Copy link
Contributor

@webknjaz webknjaz commented Dec 20, 2021

@welcome
Copy link

welcome bot commented Dec 20, 2021

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.

Welcome to the EBP community! 🎉

@codecov
Copy link

codecov bot commented Dec 20, 2021

Codecov Report

Base: 89.86% // Head: 89.86% // No change to project coverage 👍

Coverage data is based on head (7c01fc2) compared to base (28725fc).
Patch has no changes to coverable lines.

❗ Current head 7c01fc2 differs from pull request most recent head 47785de. Consider uploading reports for the commit 47785de to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #469   +/-   ##
=======================================
  Coverage   89.86%   89.86%           
=======================================
  Files          21       21           
  Lines        2150     2150           
=======================================
  Hits         1932     1932           
  Misses        218      218           
Flag Coverage Δ
pytests 89.86% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor Author

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

@chrisjsewell I've annotated some parts of the patch to make it easier for you to understand it. It's similar to how most of the projects in the wider Python community use it but with a few tiny adjustments to make it work in this one.

@@ -0,0 +1,11 @@
---
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a config for a GitHub App that is able to help with checking whether a change fragment is present in a given PR. It allows skipping this requirement by labeling a PR and for PRs created by certain robots. It needs to be installed separately from the PR but I've copied the config just in case.

@@ -1,5 +1,23 @@
# Changelog

[//]: # (DO-NOT-REMOVE-versioning-promise-START)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A marker for inclusion in Sphinx

https://github.com/executablebooks/MyST-Parser/tree/master/docs/changelog-fragments.d#adding-change-notes-with-your-prs
-->

<!-- towncrier release notes start -->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Below this marker, Towncrier will inject the release entry using the j2 template.

@@ -0,0 +1,31 @@

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the template Towncrier is set up to use. You can change virtually anything. Changing it won't affect the historic entries because Towncrier only adds new ones and doesn't touch anything that is already present in CHANGELOG.md.

@@ -0,0 +1,25 @@
*
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a safety thing, so undesired files can't be included by accident.

Comment on lines +21 to +159
[[tool.towncrier.section]]
path = ""

[[tool.towncrier.type]]
directory = "bugfix"
name = "Bugfixes"
showcontent = true

[[tool.towncrier.type]]
directory = "feature"
name = "Features"
showcontent = true

[[tool.towncrier.type]]
directory = "deprecation"
name = "Deprecations (removal in next major release)"
showcontent = true

[[tool.towncrier.type]]
directory = "breaking"
name = "Backward incompatible changes"
showcontent = true

[[tool.towncrier.type]]
directory = "doc"
name = "Documentation"
showcontent = true

[[tool.towncrier.type]]
directory = "misc"
name = "Miscellaneous"
showcontent = true

[[tool.towncrier.type]]
directory = "internal"
name = "Contributor-facing changes"
showcontent = true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is where you can specify custom types.

setup.cfg Outdated
@@ -65,12 +65,14 @@ linkify =
# Note: This is only required for internal use
rtd =
ipython
setuptools-scm ~= 6.3.2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is used for an SCM-referencing version representation in Sphinx.

setup.cfg Outdated
sphinx-book-theme~=0.1.0
sphinx-panels~=0.5.2
sphinxcontrib-bibtex~=2.1
sphinxext-rediraffe~=0.2
sphinxcontrib.mermaid~=0.6.3
sphinxext-opengraph~=0.4.2
sphinxcontrib-towncrier ~= 0.2.0a0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This allows embedding the "next release" changelog preview.

Comment on lines +40 to +41
-git fetch --unshallow
-git fetch --tags
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is necessary for setuptools-scm to calculate the distance to the closest tag.

@@ -52,6 +57,41 @@ commands =
-n -b {posargs:html} docs/ docs/_build/{posargs:html}


[testenv:make-changelog]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can use this command to preview the changelog draft on CLI or actually prepare the next release changelog entry if you pass an explicit version as mentioned in the description.

@chrisjsewell
Copy link
Member

cheers @webknjaz!
Out of the office at the moment 🎄, but will try to have a proper look next week

docs/conf.py Outdated
Comment on lines 20 to 25
github_url = "https://github.com"
github_repo_org = "ansible"
github_repo_name = "ansible-language-server"
github_repo_slug = f"{github_repo_org}/{github_repo_name}"
github_repo_url = f"{github_url}/{github_repo_slug}"
github_sponsors_url = f"{github_url}/sponsors"
Copy link
Member

Choose a reason for hiding this comment

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

Is this a copy-pasta?
Can this be all moved down with the extlinks configuration: I'm guessing that is only where it is being used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is. I have it here in all the projects because it's where the common args are defined + these can be reused in other places if necessary. Keeping such things in the same place across projects, makes sure that when I sync the configuration into various repos, there's not many conflicts, it's easier from the maintenance perspective.

from functools import partial
from pathlib import Path

from setuptools_scm import get_version
Copy link
Member

Choose a reason for hiding this comment

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

I note that setuptools_scm says "It is discouraged to use setuptools_scm from Sphinx itself": https://github.com/pypa/setuptools_scm/#usage-from-sphinx
thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds reasonable. Although, this code is what I use right now in many places and it works. Reading the version from metadata means that the project needs to be pip installed into the same venv where sphinx is. It would subsequently pull more dependencies which is not possible or desirable in some cases.
OTOH it's not possible to switch the solution right now because MyST does not use setuptools-scm to generate the distribution version — we'd not have the desired information before such a switch.
But if you want, I can contribute that separately, after this PR is in, for example.

docs/conf.py Outdated Show resolved Hide resolved
@chrisjsewell
Copy link
Member

Sorry, I will eventually get round to addressing this 😅
Its just that slight "extra level of complexity", which I'm sure is necessary, but is stopping it being a complete no-brainer for me (particularly as I've just been on a bit of a coding with myst-parser/myst-nb, and didn't want any extra steps lol)

@webknjaz webknjaz force-pushed the maintenance/towncrier branch 2 times, most recently from 7c01fc2 to c866013 Compare October 19, 2022 13:31
@webknjaz
Copy link
Contributor Author

@chrisjsewell I've updated this for you

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.

[release automation] Changelog fragments management
2 participants