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

Replace PROJECT placeholder in changes.rst for "harmonica" #341

Merged
merged 1 commit into from
Aug 23, 2022

Conversation

BenjMy
Copy link
Contributor

@BenjMy BenjMy commented Aug 19, 2022

Hey, Harmonica curious user here!

I found out that some links in the change.rst return a 404 error.

I manually replaced all the <https:/fatiando/PROJECT/pull/*> by <https:/fatiando/harmonica/pull/*>.
Is it the way to do or is there any automatic implementation behind it? anyway, I let you review the PR according to your standards ;)

Thanks for your awesome work!

Hey, Harmonica curious user here! 
I found out that some links in the `change.rst` (mentioning the issues #) return a 404 error. 
I manually replaced all the `fatiando#307 <https:/fatiando/PROJECT/pull/*>` by fatiando#307 <https:/fatiando/harmonica/pull/*>`. Is it the way to do or is there any automatic implementation behind it? anyway, I let you review the PR according to your standards ;)
Thanks for your awesome work!
@welcome
Copy link

welcome bot commented Aug 19, 2022

💖 Thank you for opening your first pull request in this repository! 💖

A few things to keep in mind:

No matter what, we are really grateful that you put in the effort to do this!

Copy link
Member

@leouieda leouieda left a comment

Choose a reason for hiding this comment

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

@BenjMy thanks for spotting this and submitting a patch! This must have slipped through the cracks in #335

We have a checklist item with a regex that does this but we've been needing to script the changelog generation for a while now (based on the checklist in that issue). Some of it has to be manual, like the sorting and summarizing of changes, but most of that could be automated with a script.

@leouieda
Copy link
Member

@santisoler should we make a 0.5.1 release to publish this fix?

@santisoler
Copy link
Member

santisoler commented Aug 23, 2022

Nice catch @BenjMy! Thank you very much for solving it! This bug is on me.

We have a checklist item with a regex that does this but we've been needing to script the changelog generation for a while now (based on the checklist in that issue). Some of it has to be manual, like the sorting and summarizing of changes, but most of that could be automated with a script.

We should add another item to the release checklist that runs sed -i 's/PROJECT/harmonica/', because this is not the first time that I forget to replace the PROJECT placeholder (the first time it happened, I think I catched it before merging).

@santisoler should we make a 0.5.1 release to publish this fix?

Sure! We can add #342 to 0.5.1 as well.

I'm merging this. @BenjMy, feel free to add yourself to the AUTHORS.md if that's something you want.

@santisoler santisoler changed the title Update changes.rst Replace PROJECT placeholder in changes.rst for "harmonica" Aug 23, 2022
@santisoler santisoler merged commit 212d9c7 into fatiando:main Aug 23, 2022
@welcome
Copy link

welcome bot commented Aug 23, 2022

🎉 Congrats on merging your first pull request and welcome to the team! 🎉

If you would like to be added as a author on the Zenodo archive of the next release, add your full name, affiliation, and ORCID (optional) to the AUTHORS.md file of this repository. Feel free to do this in a new pull request if needed.

We hope that this was a good experience for you. Let us know if there is any way that the contributing process could be improved.

@BenjMy BenjMy deleted the patch-1 branch August 24, 2022 09:22
@santisoler santisoler added the bug Report a problem that needs to be fixed label Aug 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Report a problem that needs to be fixed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants