Skip to content

Conversation

@Self-Hosting-Group
Copy link
Contributor

@Self-Hosting-Group Self-Hosting-Group commented Jan 27, 2024

and change title to term used in LuCi

Maintainer:
Compile tested: not tested on OpenWrt
Run tested: not tested on OpenWrt

@Self-Hosting-Group
Copy link
Contributor Author

This commit miniupnp/miniupnp@02da705 renamed a configuration option in the updated miniupnpd to enable_pcp_pmp, but since the old name is still accepted, no change was made in the PR.

@1715173329
Copy link
Member

but since the old name is still accepted, no change was made in the PR.

Please follow upstream change and migrate to new config name.

@BKPepe
Copy link
Member

BKPepe commented Jan 28, 2024

Also, make sure that it is compile and run tested for OpenWrt.

@Self-Hosting-Group Self-Hosting-Group force-pushed the net/miniupnpd-update-2.3.4 branch from 73f822e to 30cf014 Compare January 29, 2024 08:17
@Self-Hosting-Group Self-Hosting-Group changed the title net/miniupnpd: Update to 2.3.4 net/miniupnpd: Update package title Jan 29, 2024
@BKPepe
Copy link
Member

BKPepe commented Feb 7, 2024

Ehh... for me it seems like the title is the same. Why do we need to have this change? Since, there is no commit description, it is not clear. I am in favor to close this PR.

@Self-Hosting-Group Self-Hosting-Group force-pushed the net/miniupnpd-update-2.3.4 branch 3 times, most recently from 845e350 to d6cf6de Compare February 17, 2024 17:41
@Self-Hosting-Group Self-Hosting-Group force-pushed the net/miniupnpd-update-2.3.4 branch from d6cf6de to c7eb7fa Compare February 22, 2024 14:31
@Self-Hosting-Group Self-Hosting-Group force-pushed the net/miniupnpd-update-2.3.4 branch from c7eb7fa to 69f9d91 Compare March 5, 2024 15:35
@Self-Hosting-Group
Copy link
Contributor Author

@BKPepe

Ehh... for me it seems like the title is the same. Why do we need to have this change? Since, there is no commit description, it is not clear. I am in favor to close this PR.

Yes, almost, it is a cosmetic change, an adaptation to the revised wording of the corresponding luci-app-upnp plugin openwrt/luci#6863. PR updated to include the version update.

@stangri
Copy link
Member

stangri commented Apr 5, 2024

@Self-Hosting-Group thank you for your contribution. Since I was tagged on this PR, I have the following comments:

  1. I'm not the miniupnp maintainer, I don't think I ever contributed to it actually.
  2. There's a PKG_VERSION update in this PR, which unless you're looking at the individual commits, is not mentioned in the PR itself. Either way would be nice to have a better description of all the changes.
  3. I may be wrong here, but I thought it's encouraged to squash the commits in a PR to a single commit.
  4. While the PKG_VERSION is updated, the PKG_RELEASE is not reset to 1.
  5. I feel that the uci-defaults script to migrate the old setting a a new one would be welcome.
  6. Should this also not be linked from a corresponding PR in the luci-app to effect the change of settings?

@Self-Hosting-Group Self-Hosting-Group changed the title net/miniupnpd: Update package title net/miniupnpd: Update package to 2.3.6 Jun 17, 2024
@Self-Hosting-Group Self-Hosting-Group force-pushed the net/miniupnpd-update-2.3.4 branch from 69f9d91 to ba05b93 Compare June 17, 2024 13:04
@Self-Hosting-Group
Copy link
Contributor Author

@stangri: Thank you for your reply. PR updated. Can we possibly do the PR without the configuration change (as backword compatible) and do it in a later PR (including LuCi)? I am waiting for some more upstream changes.

@stangri
Copy link
Member

stangri commented Jun 18, 2024

@stangri: Thank you for your reply. PR updated. Can we possibly do the PR without the configuration change (as backword compatible) and do it in a later PR (including LuCi)? I am waiting for some more upstream changes.

That's up to maintainer, I just provided (hopefully helpful) feedback.

@BKPepe
Copy link
Member

BKPepe commented Jun 18, 2024

CI is still failing for you. Would you mind to fix it?

and change title to term used in LuCi

Signed-off-by: Self Hosting Group <155233284+Self-Hosting-Group@users.noreply.github.com>
@Self-Hosting-Group Self-Hosting-Group changed the title net/miniupnpd: Update package to 2.3.6 miniupnpd: Update package to 2.3.6 Jun 19, 2024
@Self-Hosting-Group Self-Hosting-Group force-pushed the net/miniupnpd-update-2.3.4 branch from ba05b93 to a2c77be Compare June 19, 2024 10:20
@neheb neheb merged commit c9a1705 into openwrt:master Jun 24, 2024
@Neustradamus
Copy link

@Self-Hosting-Group: Thanks!

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.

6 participants