Flatpak build fixes - Githubissues

leamas commented 4 years ago

This is actually very hard to read. Could you please rebase your branch against current master before making a build request?

Many (most) of the commits listed here are actually already merged...

leamas commented 4 years ago

Actyally, a git rebase upstream/master converts this mess to a single commit aed18e0.

Let's start there.

leamas commented 4 years ago

After rebasing, looking at aed18e0: what's this all about?

In other words: what is the problem solved by this commit? Could you please file a bug describing the problem you are solving, to get some context?

jongough commented 4 years ago

I did do a re base before pushing the change. I was surprised at the way git handled it, but...

The patch is to remove the plugin name from the file and replace it with the cmake known name. This removes a change needed if this whole process is to be used by other plugins. i.e. it makes it more generic. I thought that the changes being made to squiddio were going to be propagated through all the other plugins by copying the files and making as few changes as possible.

I have now moved to my testplugin_pi to do these changes as it appears there is resistance to using squiddio.

leamas commented 4 years ago

I did do a re base before pushing the change. I was surprised at the way git handled it, but...

Sorry, but frankly: no.

Please do:

 git remote add upstream https://github.com/mauroc/squiddio_pi.git
 git remote update upstream
 git rebase upstream/master

This should definitely convert this PR to a single commit on top of current upstream master.

leamas commented 4 years ago

I have now moved to my testplugin_pi to do these changes as it appears there is resistance to using squiddio.

I wouldn't say resistance. I just want to make the trade off between being generic or simple visible. In the end, this is something @mauroc needs to decide. IMHO, this is best done in a separate bug describing what you are trying to achieve.

Personally, I'm on my way out of this. As soon as #89 is resolved we should be ready to transfer the squiddio plugin artifacts to this project. Basically, that's my quest here.

BTW: I think I have managed to clean up the ci builders and deployment. Part of this was moving some configuration out of the upload scripts.

jongough commented 4 years ago

You are the one proposing/pushing this change to distribution. I am one of the ones trying to use your new method. You may find it crystal clear, but to me it is very murky and I am having difficulty understanding your changes and how to apply them to my plugins.

We had already moved to using travis and appveyor for build and that added complexity and a whole new set of documentation was required. The process has now gone a step further and is more complex again. The documentation has not caught up and is difficult, for me anyway, to follow and understand.

I am trying to make the process easier for other plugin developers so that they don't have to go through such a steep learning curve. At the moment it is still a 'work in progress' and therefore leaving it 'as is' is not helpful. You need to continue with this until it is stable, usable by standard developers and automated as much as possible. It then needs comprehensive, step-by-step, instructions with pictures so that others can use it.

I have made the offer of my 'developer' plugin to do this work in an area that is usable but will not impact users of OCPN. The offer still stands.

jongough commented 4 years ago

Closed and created new pull request

leamas commented 4 years ago

I am trying to make the process easier for other plugin developers so that they don't have to go through such a steep learning curve.

Which is fine, really. There is a small gotcha, though: Making things generic adds complexity. That is not to say that it is wrong in all (or this) situation. It's just not simple.

Here is a basic trade-off: keeping it focused on squiddio, or making the code a great starting point for other devs. I cannot judge what is the proper way here, it's just not my decision to make.

In the end, IMHO this is something @mauroc should decide, probably best in a separate bug like "Make squiddio a generic starting point for other devs" or so.

However, there is another string to pull. One might certainly file a bug like "Configuration spread into too many places". And make changes with this in focus. Having most of the configuration in CMakeLists.txt is of course big advantage (even though I doubt that flatpak/*yaml should be handled this way). There are some steps in this direction in #101.

rgleason commented 4 years ago

I am trying to make the process easier for other plugin developers so that they don't have to go through such a steep learning curve. At the moment it is still a 'work in progress' and therefore leaving it 'as is' is not helpful. You need to continue with this until it is stable, usable by standard developers and automated as much as possible. It then needs comprehensive, step-by-step, instructions with pictures so that others can use it.

Jon I appreciate this effort. Sometimes these efforts are trying to play catch up with Alec's fast moving changes which make it more difficult. It appears much of the build and deploy structural changes have been completed by Alec at this point. There will be continuing improvements, but this aspect will get easier.

I do think that improvements to make deployment to all plugins easier are well worthwhile. I know I will appreciate it. I have been trying to follow and document these efforts here. If you have thoughts or suggestions for documentation, please advise me! https://opencpn.org/wiki/dokuwiki/doku.php?id=opencpn:developer_manual:pi_installer_summary https://opencpn.org/wiki/dokuwiki/doku.php?id=opencpn:developer_manual:pi_installler_build_deploy This last page needs some updating.

I was looking at testplugin:cmake_flatpak and was interested in all the commits https://github.com/jongough/testplugin_pi/tree/cmake_flatpak

mauroc / squiddio_pi

Flatpak build fixes #104