Skip to content

Conversation

@ann0see
Copy link
Member

@ann0see ann0see commented Mar 2, 2022

Short description of changes
Moves some files around to be consistent with Windows/Android build
Removes unneeded files

Changelog entry might need to be split.

Context: Fixes an issue?
Fixes: #2430

Does this change need documentation? What needs to be documented and how?

Status of this Pull Request
Finished
What is missing until this pull request can be merged?
Needs a second review

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

@ann0see ann0see force-pushed the patch/removeUnneeded branch from 38d8250 to f07e040 Compare March 2, 2022 19:50
@ann0see ann0see force-pushed the patch/removeUnneeded branch 4 times, most recently from b7a5892 to 31049ea Compare March 2, 2022 20:12
@ann0see ann0see force-pushed the patch/removeUnneeded branch 3 times, most recently from fd0cf18 to 4ffa613 Compare March 2, 2022 20:20
@ann0see ann0see force-pushed the patch/removeUnneeded branch 3 times, most recently from 5e15e19 to 2256711 Compare March 2, 2022 20:29
@ann0see
Copy link
Member Author

ann0see commented Mar 2, 2022

Sorry for the development noise here. Will set it to draft.

@ann0see ann0see marked this pull request as draft March 2, 2022 20:32
@ann0see ann0see force-pushed the patch/removeUnneeded branch from 1187a81 to 40ca392 Compare March 2, 2022 20:37
Copy link
Member

@hoffie hoffie left a comment

Choose a reason for hiding this comment

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

Looks good to me now. Two open questions:

  • Flatpak is removed because... there's noone on the team maintaining it / using it to publish updates?
  • I think the non-autobuild debian logic removal is fine. Maybe still ping the official package maintainer and ask about it to be sure? (mirabilos)

@ann0see
Copy link
Member Author

ann0see commented Mar 2, 2022

Flatpak is removed because... there's noone on the team maintaining it / using it to publish updates?

Exactly. Although I have access to the flatpak repo (you probably also have) it's not official. Therefore I don't think it makes sense having it here and on the repo of flatpak.

@ann0see
Copy link
Member Author

ann0see commented Mar 2, 2022

I think the non-autobuild debian logic removal is fine. Maybe still ping the official package maintainer and ask about it to be sure? (mirabilos)

I remember him saying that debian uses its own logic anyways.

@mirabilos could you please comment if modifying the debian folder does any harm for you? Probably the debian build in their repo is built different anyway.

@ann0see ann0see marked this pull request as ready for review March 2, 2022 21:03
@mirabilos
Copy link
Contributor

mirabilos commented Mar 2, 2022 via email

@ann0see
Copy link
Member Author

ann0see commented Mar 2, 2022

Ok. So you're basically not dependent on anything this PR is doing.

@ann0see
Copy link
Member Author

ann0see commented Mar 2, 2022

@hoffie I installed the resulting deb and it seemed to work fine. However the text description in Gnome software seemed wring (but that's not important at all)

@ann0see ann0see added this to the Release 3.9.0 milestone Mar 2, 2022
@pljones
Copy link
Collaborator

pljones commented Mar 3, 2022

Can I ask for a quick feature request as part of this? Can you add a Jamulus Server .desktop file to the Linux .deb? Or doesn't it fit easily here?

@hoffie
Copy link
Member

hoffie commented Mar 3, 2022

However the text description in Gnome software seemed wring

In what way?

Can I ask for a quick feature request as part of this? Can you add a Jamulus Server .desktop file to the Linux .deb? Or doesn't it fit easily here?

I think it'd be good to keep "Refactor" PRs to be exactly that. Refactoring. :)

@mirabilos
Copy link
Contributor

mirabilos commented Mar 3, 2022 via email

@pljones
Copy link
Collaborator

pljones commented Mar 3, 2022

Why would you even install the headless server if you have a system that’s using .desktop files in the first place?

Where did I mention headless server?

@ann0see
Copy link
Member Author

ann0see commented Mar 3, 2022

In what way?

The gnome software output didn’t say what is written as description but something else sounding like a description. I don’t know exactly what it said.

@ann0see
Copy link
Member Author

ann0see commented Mar 4, 2022

Ok. See

grafik

I assume it's from the Debian repository, not the deb file

@ann0see
Copy link
Member Author

ann0see commented Mar 4, 2022

Can I ask for a quick feature request as part of this? Can you add a Jamulus Server .desktop file to the Linux .deb? Or doesn't it fit easily here?

I thought @jujudusud did that once (and somehow it disappeared). Could you please open a feature request on that?

@hoffie
Copy link
Member

hoffie commented Mar 4, 2022

Ok. See

grafik

I assume it's from the Debian repository, not the deb file

This seems to match the man page: https://manpages.debian.org/testing/jamulus/Jamulus.1.en.html
This is part of the tree; https://github.com/jamulussoftware/jamulus/blob/master/distributions/Jamulus.1

I thought @jujudusud did that once (and somehow it disappeared). Could you please open a feature request on that?

I agree it should be a separate issue. The file is still there, btw, it's just missing from debian/jamulus.install. I've got a test build running. If it works, I'll send a PR, if it doesn't, I'll open the extra issue.

@ann0see
Copy link
Member Author

ann0see commented Mar 7, 2022

@pljones could you please review this PR? I think we should merge it soon (to enable hoffie starting to work on the CI refactoring)

Copy link
Collaborator

@pljones pljones left a comment

Choose a reason for hiding this comment

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

Looks fine to me.

@ann0see ann0see merged commit 17aca28 into jamulussoftware:master Mar 8, 2022
@ann0see ann0see deleted the patch/removeUnneeded branch March 8, 2022 17:16
pgScorpio pushed a commit to pgScorpio/jamulus that referenced this pull request Mar 28, 2022
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.

Build: Consider merging debian/control into a single file

4 participants