What's wrong with my package?

For Developers
1

Hi folks

So about a week ago I submitted a pull request for my package (geoip-shell) to the packages repository. Up to this moment, the package is waiting for review. There are no comments at all (besides my own comment) and it looks like the expected checks have not been performed on it.

Now after a quick check in the history of this repository, I'm noticing that the typical time between PR for new package and first reply is 0-1 days. So I'm getting the impression that there is something wrong with my package. Do you guys mind telling me what?

I don't mean to pressure anyone, I just want to understand what's the reason for what appears to be an unusually long time without any progress and what my expectations should be going forward.

Here's the link, for convenience:

https://github.com/openwrt/packages/pull/23821

2

I get it: I want my mistakes called out too.

What’s wrong with my package?

But you must admit: that is too punny! :face_holding_back_tears:

2 Likes
3

Haha, sorry about that - pun definitely not intended (and I'm not a native English speaker, so it even took me a moment to figure out what's the joke about).

1 Like
4

https://forum.openwrt.org/t/geoip-shell-flexible-geoip-blocker-for-linux-now-supports-openwrt/189611
You have already moved the conversation from github to the forum.

For the other part about review I don’t know about the timescale you talk about. This process take the time it takes.
And you need to find reviewers that need the function for the package to actually get them to build it and test it.

But you do have fails on the auto check…
I can guarantee nothing will happen before you have complied to the auto check.

You don’t want to give us your real name, but you still have copyright demands (more on that at the end) and now you come and complain we work slow.
Ohh, my. You must look up and see the news more. Have you heard about the “Linux xz hack”?

Since you are sending in commits in a continuous line it doesn’t look like the code is ready for review.

The commit message must at the bottom contain a “signed off by” line.
Don’t use links in the commit message, they don't work over time. Write the information that is needed in the commit message. It will live for ever, the links don’t.
You don’t say who is the maintainer in the commit message. That is defined in the package make file.

I would change the branch from master to main, master isn’t used anymore. It is only a mirrored branch.

You can’t have copyright lines in combo with gnu licenses. And all these licenses claims are supposed to be defined in the commit message.

3 Likes
5

Dear @flygarn12, first off, is using aggressive language like "no on gives a f*ck" a norm here? This is the first time I've seen this sort of expression on this forum. Please educate me on this subject.

Now about me not wanting to give you my real name. I signed off all my commits with it. So where does this claim even come from? The fact that I didn't use it in the Makefile? Well, first, where's the rule that says that this is even necessary? Second, I asked here, on this forum, whether I'm supposed to do that and I didn't get any authoritative answer. So how am I expected to even figure this out?

About copyright "demands" - please cite any demands I've made. The "copyright: antonk" is simply there to identify the author. How does this contradict the GPL license?

Also, please cite any complains I've made as I'm not aware of any.

About links, I didn't know that it's no good - sure, I'll remove them.

As to sending commits, geoip-shell is being actively developed as lots of software out there. What's wrong with adding commits when the review process hasn't even started? Again, I asked about this specifically on this forum and the answer I've got is that it's fine. Anyway, the code is absolutely ready for review.

About the auto check, I'll be happy to comply but i don't understand how to do that. Am i supposed to create a Github account with my official name? Probably not going to happen. Also, i see that most contributors don't actually do this. On the other hand, the CONTRIBUTING.md file says that I should sign off commits with my official name. Which is what I did. This, however, to my understanding, triggered the autocheck to fail because the signoff name doesn't match the Github account name. So what should I do to fix that?

As to finding reviewers who are interested in the project. I don't know too many people here and i certainly don't know who is authorized to review packages. I'm offering the project as a contribution to the community and i know that some people will use and appreciate it. These may or may not be people with review authorization. I've put a fair amount of work into adapting it to OpenWrt. I'm not going to be begging for it to get reviewed. Especially I'm not going to be begging people who speak in a hostile and condescending manner like you do. I also understand that nobody owes me anything.

6

Absolutely not and it is forbidden in the ToS.
I apologize for liking the post; I, clearly, missed that.

2 Likes
7

If you look at your commits in https://github.com/openwrt/packages/pull/23821/commits (you also seem to have a merge commit, that should be removed), every single one of them needs to adhere to the formalitities, so in your case each and every commit subject should start with geoip-shell: , just as you did correctly for commit ebb9fea3b2518f8b6a2852bc8bcf9ee3f35c38c6.

Everyone is allowed to review but only maintainers with higher privileges are allowed to merge the commits. Of course, by having more reviews the maintainers are gaining bigger confidence that the changes being made is correct, increasing the chance of getting it merged. That's just kinda how open source works in my experience, for better and worse.

4 Likes
8

Thank you. What's the best course of action to fix the issue with the commit subject? Force pushing changes to commits? Or canceling this pull request and making a new one?

9

OMG NO!!!!

Don't change the topic/pull request title!!!! :rofl: :hugs:

Well, maybe.... :sleeping:

10

git rebase && fast forward to current HEAD and force push, yes.

The name of the pull request does not end up in the git history, so that can be changed (as long as it remains recognizable). What should be avoided, is closing the PR and filing a new one, as that voids previous reviews and gets confusing fast (there may still be reasons to do so, but avoid, if reasonably possible).

Disclaimer: this is just general advice, I can't help you with merging the PR.

5 Likes
11

I agree with @slh but in this case maybe start over.
Take note of : https://openwrt.org/submitting-patches
Strictly adhere to it

I have made a few PR's and it sometimes takes time before the maintainers are able to review the PR.
New packages need 6 reviews, you can ask in the forum for people to review your package

1 Like
12

Thank you @egc , @slh , @dannil for your help. I ended up doing git rebase while fixing (hopefully) all the issues with the formalities. I'll post a request for reviews later, but before that I'd like to confirm that the formalities are good now. Waiting for a maintainter to run formalities autocheck.

13

Now that you have edited out the most offending part of your post, I can actually focus closely on some details which I didn't quite read deeply into on the first read.

About copyright. As I wrote above, the purpose is simply identifying the author. You could say that I am claiming my authorship for this code. I did put this in the Makefile, right above the license line because looking at some other Makefiles of merged packages, I've seen this done exactly this way.

For example ('pbr' package):

# Copyright 2017-2022 Stan Grishin (<email-edited-out>)
# This is free software, licensed under the GNU General Public License v3.

Considering the language that you chose to use in your post, and you misrepresenting my words and intentions ("You don’t want to give us your real name", "you still have copyright demands", "you come and complain we work slow"), I'm really unsure about the other parts of your post - whether they are another expression of your obvious attitude towards me personally (for the record, I have no idea where this attitude comes from as this was our first interaction), or if they actually have a basis in reality. I really do want to comply to rules which are based in reality, so please clarify about your other claims, including the claim about mixing copyright lines and the GPL license line.

14

Also about the maintainer in the commit message. When adding a pull request, you get a text box pre-filled with the following text:

Maintainer: me / @\<github-user> (find it by checking history of the package Makefile)
Compile tested: (put here arch, model, OpenWrt version)
Run tested: (put here arch, model, OpenWrt version, tests done)

Description:

So naturally I wrote who the maintainer is in that box. Now looking at the above quote from your post, I again question what the actual rule is, and if the rule is "You don’t say who is the maintainer in the commit message" then why does that text box have this line.

15

Thank you for your answer. What is actually meant by the word "review"? If I ask users of my application to report if it's working correctly, does this count as a review? (Currently I am offering ipk packages for geoip-shell on project's Github releases page and I know that some people are using them.) Or is code review implied? And if so then what aspects of the code should be checked? Are there official guidelines for this?

16

Sure you can. You can hold the copy right and grant usage under a gnu license or another copy left license. That's the whole point.
You see, in an non Anglo country like Germany you are even unable to give away your Autorschaft, the fact that you have written a piece of text. You can only give up / away rights on usage / grant usage.
/Nitpick