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

Tuner - new package #39774

Closed
wants to merge 10 commits into from
Closed

Tuner - new package #39774

wants to merge 10 commits into from

Conversation

MechDR
Copy link

@MechDR MechDR commented Oct 6, 2022

Testing the changes

  • I tested the changes in this PR: YES

Sorry, my mistake...
Sorry for the previous commits, they were a mistake... I'm kinda new :D.
@paper42 paper42 added the new-package This PR adds a new package label Oct 6, 2022
@misuchiru03
Copy link
Contributor

I noticed when it finally built, the file names it provides are gross: com.github.louis77.tuner, and all other files where the binary name of tuner would suffice follows this naming.

@classabbyamp
Copy link
Member

the inverse-DNS naming scheme is fine. there's no need to change it

version=1.5.1
revision=1
build_style=meson
hostmakedepends="gettext pkg-config vala"
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems glib-devel is needed as hostmakedepends as well to get the glib-compile-resources on cross targets

Copy link
Author

@MechDR MechDR Oct 25, 2022

Choose a reason for hiding this comment

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

I'm sorry, I'm kinda new to all this... are my last commits OK? :-\

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be good, with a few remarks:

  • any blank lines are unnecessary
  • we usually indent wrapped lines like the makedepends with only one space

As mentioned below we do one commit per package, so you need to squash all these. In the future, you can use git commit --amend to add changes without introducing new commits.

Copy link
Author

Choose a reason for hiding this comment

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

  • any blank lines are unnecessary

I believe I had only one blank line at the end... POSIX standard I believe, everything ends with a new line... please correct me if I'm wrong :).

  • we usually indent wrapped lines like the makedepends with only one space

As in not do a bunch of spaces to align the indent? OK, no problem, feel free to correct the template and I will remember this for future commits ;).

As mentioned below we do one commit per package, so you need to squash all these. In the future, you can use git commit --amend to add changes without introducing new commits.

I still haven't connected my main rig with GitHub, I'm still working on the web interface :-\... I know, I'll get on it as soon as possible.

Now, how do I squash the commits :). Might be a stupid question, I know, but as I said, I'm kinda new :).

Copy link
Contributor

Choose a reason for hiding this comment

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

Now, how do I squash the commits

See "Squashing commits" in https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History, should be pretty clearly described (beware, git rebase -i sends you to the default editor, which might be vim on linux 😛)

Copy link
Author

@MechDR MechDR Oct 25, 2022

Choose a reason for hiding this comment

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

(beware, git rebase -i sends you to the default editor, which might be vim on linux 😛)

Hahahaha, good one :D. It's vi in my case I believe, it came preinstalled :D. Can't work it if my life depended on it xD.

Can't seem to find a way to squash the commits from this PR from the web UI, apparently a maintainer has to approve me since I'm a first-time contributor, and then another drop down menu appears... at least that's what I gathered from what I read :-\.

SHOT0070

Copy link
Member

Choose a reason for hiding this comment

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

That's easy to solve, don't use the web ui.

@tranzystorekk
Copy link
Contributor

Also these commits should be squashed together into one New package: tuner-1.5.1

Added geocode-glib-libsoup2-devel
Added geocode-glib-libsoup2-devel to makedepends
Added glib-devel for cross compiling
@MechDR
Copy link
Author

MechDR commented Oct 25, 2022

Are these commits OK?

Also, it states the version is 1.5.0, but it's actually 1.5.1, the author just forgot to change the version number in the About.

revision=1
build_style=meson
hostmakedepends="glib-devel gettext pkg-config vala"
makedepends="geoclue2-devel geocode-glib-devel geocode-glib-libsoup2-devel
Copy link
Member

Choose a reason for hiding this comment

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

you are mixing libsoup2 and libsoup3 versions of geocode-glib here, remove the libsoup3 one (geocode-glib-devel) or switch the whole package to libsoup3 if possible.

Copy link
Author

@MechDR MechDR Oct 27, 2022

Choose a reason for hiding this comment

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

Is this OK?

Can't switch to libsoup3, the package requires libsoup2.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, do you know if there is a plan to support libsoup3?

Copy link
Author

@MechDR MechDR Oct 31, 2022

Choose a reason for hiding this comment

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

Yes, do you know if there is a plan to support libsoup3?

No, I'm not in contact with the author. Would you like me to ask him/her?

EDIT: I asked on the GH project page, will post the reply.

louis77/tuner#103

@MechDR MechDR deleted the branch void-linux:master October 31, 2022 15:16
@MechDR MechDR closed this Oct 31, 2022
@MechDR MechDR deleted the master branch October 31, 2022 15:16
@MechDR MechDR restored the master branch October 31, 2022 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-package This PR adds a new package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants