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

Improve meson build #31

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

NathanBnm
Copy link

I made a few improvements to your app structure. These are not breaking changes. Let me know if you have any questions 😄

Changelog:

  • Renamed COPYING to LICENSE
  • Improved meson.build files
  • Added some comments to meson.build files
  • Improved and simplified post_install.py script

I would be happy to add French translation once this will have been reviewed

I tried to build and run the app locally using both meson and flatpak and I had no issues

Copy link
Contributor

@Antolius Antolius left a comment

Choose a reason for hiding this comment

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

I really like the split of large meson.build into separate files!
I left a few comments mostly regarding file naming and a few questions.

@@ -1,4 +1,4 @@
<img align="left" width="64" height="64" src="https://raw.githubusercontent.com/starfish-app/starfish/main/data/icons/64.svg">
<img align="left" width="64" height="64" src="https://raw.githubusercontent.com/starfish-app/starfish/main/data/icons/64/hr.from.josipantolis.starfish.svg">
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer not to have project name repeated in too many places, such as icon file names, etc.
Ideally it would only be defined in the project function call on top of the root meson.build file.
Individual files can be renamed in calls to install_data function. That approach allows for:

  1. Shorter file names without repetition. This just makes working with them more comfortable.
  2. Easier rename of the project in the future. Either if someone wanted to maintain a fork long term, or just easily install 2 versions on the same system.

Admittedly, I haven't followed this rule in all the places myself, but I plan to clean those up over time.

@@ -89,7 +89,7 @@ flatpak run hr.from.josipantolis.starfish

## License

[GNU GPLv3](COPYING)
[GNU GPLv3](LICENSE)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think COPYING is the standard / recommended name for the file containing the full GPL license text.
I'd like to stay aligned with that standard.

'test/Core/GeminiBodyTest.vala',
'test/Core/UriTest.vala'
# Compiling resources
gresource = gnome.compile_resources (
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to move this into data/meson.build as well?

meson/post_install.py Show resolved Hide resolved
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