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

Briefcase version metadata #425

Merged
merged 8 commits into from Jun 17, 2020

Conversation

ForeverWintr
Copy link
Contributor

Hi! I think this is a really cool project, and hope this small contribution will be of use. I intend it to resolve #412 by adding Briefcase-Version to app metadata. It can be retrieved as in your example in the linked issue.

Feedback is welcome.

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

This looks great - thanks for the contribution! The code and test is great - exactly what I was expecting to see.

All that is missing is some documentation. I'd suggest two things are needed:

  1. A new HowTo page for 'Accessing Briefcase packaging metadata at runtime'. This should give a short example for how to access runtime properties (using importlib.metadata); and list the metadata that is available at runtime.

  2. A FAQ entry for the specific question that prompted this feature request ("How do I detect if my app is running in a Briefcase-packaged container?") that directs to the HowTo page.

If you're not confident in your tech writing skills, don't worry about that too much - we can always revise the text during the review process.

@ForeverWintr
Copy link
Contributor Author

Thanks @freakboy3742! I've taken a crack at the documentation, what do you think?

I'm not sure what best practice is for providing backwards compatibility in example code. I've indecisively included both your backwards compatible metadata import, and then printed with f-strings...

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

Looks great - thanks! The example code you've provided is about as good as we can get for the general case. It might be worth mentioning that the example is "runs on all Pythons" compliant, and can be simplified if you're targetting a specific version of Python (which apps often will be). We also (unfortunately) need to avoid f-strings for now, because we still support Python 3.5.

Don't worry to much about making those changes, though; I'll make a couple of light language edits before land the patch, and I can drop in some extra content about how the example code can be simplified if you're targetting a single Python version.

@codecov
Copy link

codecov bot commented Jun 17, 2020

Codecov Report

Merging #425 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted Files Coverage Δ
src/briefcase/commands/create.py 97.24% <100.00%> (+0.01%) ⬆️

@freakboy3742 freakboy3742 merged commit 6b2cdc5 into beeware:master Jun 17, 2020
@ForeverWintr ForeverWintr deleted the briefcase-version-metadata branch June 18, 2020 00:13
@ForeverWintr
Copy link
Contributor Author

Great, thanks for the review and edits!

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.

Add a mechanism for detecting if an code is running as a bundled app
2 participants