Skip to content

Use non-scaled pen default for +p #5265

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

Merged
merged 4 commits into from
May 26, 2021
Merged

Use non-scaled pen default for +p #5265

merged 4 commits into from
May 26, 2021

Conversation

maxrjones
Copy link
Member

Description of proposed changes

This PR changes the default pen for +p to be thicker,black rather than scaled based on the size.

Several tests fail, but for the better. I can push new postscript files after review of the fixed default.

Fixes #4955 and relates to GenericMappingTools/pygmt#1287.

Fixes #

Reminders

  • Make sure that your code follows our style. Use the other functions/files as a basis.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Describe changes to function behavior and arguments in a comment below the function declaration.
  • If adding new functionality, add a detailed description to the documentation and/or an example.

@maxrjones maxrjones requested a review from PaulWessel May 26, 2021 16:11
Copy link
Member

@PaulWessel PaulWessel left a comment

Choose a reason for hiding this comment

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

Maybe we should add (void) in front of gmt_getpen since we are discarding the return value sinc we know there will be no error as the arg is a constant. One less compiler warning probably.

@maxrjones
Copy link
Member Author

Maybe we should add (void) in front of gmt_getpen since we are discarding the return value sinc we know there will be no error as the arg is a constant. One less compiler warning probably.

OK, done in eb9390f.

OK to merge after updating the PS files? I think this should be considered a 6.2.0rc2 bug that needs to be fixed for 6.2.0.

@PaulWessel
Copy link
Member

So I am unclear on how this all work. We release rc2 waiting for feedback. GMT bugs probably follows a Poisson distribution and they come in at odd intervals. if we wait long enough bugs will be reported and we then should fix them and do rc3? It is an never-ending cycle then. I have fixed other bugs and labelled them WIP. I mean. I am OK with merging those bug fixes and get 6.2.0, but if we do an rc and wait a few days then we are back at it again. Thoughts?

@maxrjones
Copy link
Member Author

So I am unclear on how this all work. We release rc2 waiting for feedback. GMT bugs probably follows a Poisson distribution and they come in at odd intervals. if we wait long enough bugs will be reported and we then should fix them and do rc3? It is an never-ending cycle then. I have fixed other bugs and labelled them WIP. I mean. I am OK with merging those bug fixes and get 6.2.0, but if we do an rc and wait a few days then we are back at it again. Thoughts?

I would prefer to release 6.2.0 next rather than do another RC. I think it's reasonable to merge bug fixes between the final RC and full release, but I could be missing something about the process since this is the first release cycle that I have been involved in.

@maxrjones maxrjones added the bug Something isn't working label May 26, 2021
@PaulWessel
Copy link
Member

What say you, @joa-quim and @seisman ? Do we merge the bug fixes or do we stop and just release 6.2.0, and after what waiting period?

@joa-quim
Copy link
Member

I have raised a couple times before the question on why we were releasing an RC2 instead of 6.2.0 but got no attention.
RCs trail around during about a week and if no regressions or serious bugs are found they become the final release. Leaving them at open during one month leads to the current situation where normal bug fixes start to pile up. I think it's silly to leaving them out of the 6.2 release which should be released almost ASAP.

@PaulWessel
Copy link
Member

Given that once we release 6.2.0 there won't be a quick release right away, so leaving known bug fixes in the github as WIP does not seem like a good idea to me. I think we should merge in bug fixes now.

@PaulWessel PaulWessel merged commit a667c42 into master May 26, 2021
@PaulWessel PaulWessel deleted the inset-pen branch May 26, 2021 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Potential issues or breaking changes with the GMT modern theme?
3 participants