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 a changelog checker script #382

Merged
merged 7 commits into from
Sep 22, 2021
Merged

Add a changelog checker script #382

merged 7 commits into from
Sep 22, 2021

Conversation

reivilibre
Copy link
Contributor

No description provided.

.github/workflows/changelog_check.yml Outdated Show resolved Hide resolved
scripts-dev/check_newsfragment.sh Outdated Show resolved Hide resolved
reivilibre and others added 2 commits September 7, 2021 10:22
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

# Print a link to the contributing guide if the user makes a mistake
CONTRIBUTING_GUIDE_TEXT="!! Please see the contributing guide for help writing your changelog entry:
https://github.com/matrix-org/sygnal/blob/main/CONTRIBUTING.md#changelog"
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we using Sygnal's contributions guidelines on purpose? Shouldn't we copy them over to Sydent instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copying them wouldn't make sense, and didn't want this to get blocked behind writing new ones

Copy link
Member

Choose a reason for hiding this comment

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

I hadn't noticed this. I think it might be better just not to write anything than to link to the CONTRIBUTING from another project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For what it's worth, in the PR template we link to Synapse's :D.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've put up a PR (#393) which includes enough of the contributing guide to include the changelog entry guide. (I think it's fairly important to have a reference for it, because I doubt I would personally know what to do if I didn't already know from Synapse/Sygnal).

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

looks generally fine otherwise


# Print a link to the contributing guide if the user makes a mistake
CONTRIBUTING_GUIDE_TEXT="!! Please see the contributing guide for help writing your changelog entry:
https://github.com/matrix-org/sygnal/blob/main/CONTRIBUTING.md#changelog"
Copy link
Member

Choose a reason for hiding this comment

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

I hadn't noticed this. I think it might be better just not to write anything than to link to the CONTRIBUTING from another project.

# 1: the newsfragment is wrong in some way
# 9: the script has not been invoked properly

echo -e "+++ \033[32mChecking newsfragment\033[m"
Copy link
Member

Choose a reason for hiding this comment

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

bit odd to mix \033 and \e in the same file, but 🤷‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I nicked it from Synapse, but good spot. I'll use \e for both (TIL that there's \e for escape!) for the sake of doing the right thing :).

@reivilibre reivilibre merged commit 7a60521 into main Sep 22, 2021
@reivilibre reivilibre deleted the rei/newsfile_checker branch September 22, 2021 13:57
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.

3 participants