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

Change the organisation name #51

Merged
merged 2 commits into from
Feb 8, 2024
Merged

Change the organisation name #51

merged 2 commits into from
Feb 8, 2024

Conversation

dcaliste
Copy link
Collaborator

@dcaliste dcaliste commented Feb 7, 2024

Change the organisation name to sailfishos-applications. Also change the QSettings objects to use the application organisation name.

Add some code to automatically migrate the data from the old directories to the new ones, defined by #21 and by changing the organisation name.

This should fix #50.

Before accepting this MR, we need to agree on the organisation name. Your proposition to use the github location looks fine. I'm just wondering if the code would be used one day outside of Sailfish (need to rewrite the UI, I know...), does this organisation name still makes sense ? Migration code are a bit error prone to write :-/

Change the organisation name to sailfishos-applications.
Also change the QSettings objects to use the application
organisation name.
Move the cache, settings and database from
the old locations to the new ones.
Copy link
Contributor

@Olf0 Olf0 left a comment

Choose a reason for hiding this comment

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

LGTM, with my quite limited C++ knowledge and experience.

@Olf0
Copy link
Contributor

Olf0 commented Feb 7, 2024

Your proposition to use the github location looks fine. I'm just wondering if the code would be used one day outside of Sailfish (need to rewrite the UI, I know...), does this organisation name still makes sense ?

My points are:

  • FlowPlayer is mostly about UI, as today every desktop (Gnome, KDE etc.) comes with a MediaPlayer.
  • FlowPlayer is extremely QML-heavy, so the only target it may technically make sense to port to is KDE Plasma Mobile; still one would have to replace all Silica calls with new calls and likely also adaption-code.
  • Even if FlowPlayer becomes ported to a non-SailfishOS platform, I see no reason why its origin should be hidden: It is a ported SailfishOS-application then.

The second option would be to use flowplayer as organisation, as many desktop- and SailfishOS-apps do; but as you stated in issue #47, organisation == application is a bit silly duplication.

Migration code are a bit error prone to write :-/

Thank you, that you did write quite some migration code: This is really nice to users. 😃
As stated before, as the first version field always has been zero and all v0.3.x releases have been marked as release candidates, IMO a prominent notice about the (again) altered DB & Config locations would have been sufficient, hence no migration code strictly necessary.

@Olf0
Copy link
Contributor

Olf0 commented Feb 8, 2024

Before accepting this MR, we need to agree on the organisation name.

Did we discuss that sufficiently in your opinion?
BTW, I am out of additional arguments to the ones I named.

If this is fine for you, I will merge this PR tonight.

@dcaliste
Copy link
Collaborator Author

dcaliste commented Feb 8, 2024

Sorry, I didn't reply. I'm fine with your arguments. No objection ! I just wanted to be sure that the decision was clear and its pros and cons discussed. It seems it is. So let's go…

@Olf0 Olf0 merged commit aeb427e into devel Feb 8, 2024
1 check passed
@Olf0 Olf0 deleted the config branch February 8, 2024 17:43
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.

[Bug] database and configuration file are not under the same directories in ~/.config/
2 participants