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 silly bug in PR #1782. #2044

Merged
merged 1 commit into from
Dec 25, 2021
Merged

Fix silly bug in PR #1782. #2044

merged 1 commit into from
Dec 25, 2021

Conversation

trss
Copy link

@trss trss commented Dec 24, 2021

make release ends up with a note saying xz isn't installed even when it is and has been used without falling back to bz2.

Same fix applies to 2.x branch also. Requesting the person merging this PR to fix that too. Not creating a separate PR for that.

Fixes #1782

Checklist:

  • My change is being tested and reviewed against the Sparkle 2.x branch. New changes must be developed on the 2.x development branch first.
  • My change is being backported to master branch (Sparkle 1.x). Please create a separate pull request for 1.x, should it be backported. Note 1.x is feature frozen and is only taking bug fixes, localization updates, and critical OS adoption enhancements.
  • I have reviewed and commented my code, particularly in hard-to-understand areas.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • My change is or requires a documentation or localization update

Testing

I tested and verified my change by using one or multiple of these methods:

  • Sparkle Test App
  • Unit Tests
  • My own app
  • Other (please specify)

Tested build release with xz installed. Didn't test without xz installed but it should trivially work. Requesting the person merging this PR to verify by eyeballing the change and/or by testing without xz installed.

macOS version tested: 10.15.7

@zorgiepoo zorgiepoo merged commit f453625 into sparkle-project:master Dec 25, 2021
@zorgiepoo
Copy link
Member

Thanks, silly/harmless bug indeed. Verified warning is outputted on my machine that doesn't have xz installed.

@trss
Copy link
Author

trss commented Dec 27, 2021

Update 1.27.1 tag?

@zorgiepoo
Copy link
Member

zorgiepoo commented Dec 27, 2021

I don't consider this issue to be important enough on its own to tag a 1.27.2 release (altering the existing 1.27.1 tag will not work well..). It's not a bug in the framework and not even something most people would hit trying to integrate Sparkle (because they don't compile Sparkle from source, or they compile the framework source as a subproject instead of make release. So it's not very user-facing). Even if it is hit, the incorrect log is harmless (and it relies on xz being installed).

If there are more changes to bring back to 1.x that are critical enough to do another 1.x release (like framework regressions for example, considering that 2.0 was just released), this change will come along the next tag. Until then, the change is on top of 1.x branch and incorporated in 2.0.0.

@trss
Copy link
Author

trss commented Dec 27, 2021

Definitely not 1.27.2, yes. Updating 1.27.1 doesn't work well, right. New to contributing so thanks for the heads up on these, and also on fixing my commit email address.

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.

2 participants