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

vpn-policy-routing: update to 0.3.5-1 #16144

Merged
merged 1 commit into from Jul 27, 2021
Merged

vpn-policy-routing: update to 0.3.5-1 #16144

merged 1 commit into from Jul 27, 2021

Conversation

stangri
Copy link
Member

@stangri stangri commented Jul 18, 2021

Maintainer: me
Compile tested: x86_64, Sophos SG-105, 21.02.0-rc3
Run tested: x86_64, Sophos SG-105, 21.02.0-rc3, start/stop

Description:
support for 21.02.0-rc2 and up
support for reloading a single interface on ifup/ifupdate
rename config file
updated shellcheck compatibility
remove obsolete create/remove_lock
interface processing optimizations to speed up reloads
drop dependency on curl in user scripts
uniform styling of functions

Signed-off-by: Stan Grishin stangri@melmac.net

@stangri
Copy link
Member Author

stangri commented Jul 21, 2021

I don't see a way to manually run the Test Build workflow, please advise.

@stangri
Copy link
Member Author

stangri commented Jul 23, 2021

@jow- @aparcar @neheb could you please advise if this can still be merged despite the two platforms build fail or if I need to do something to rectify it? Is there any way I can cause a rebuild without force-pushing a meaningless change?

PS. Even tho I've got the merge rights for luci pretty quickly, I've been asking for merge rights for packages for over a year now, can someone please explain to me what the process is and what should I do get things moving?

@aparcar
Copy link
Member

aparcar commented Jul 23, 2021

PS. Even tho I've got the merge rights for luci pretty quickly, I've been asking for merge rights for packages for over a year now, can someone please explain to me what the process is and what should I do get things moving?

Who did you ask? Maybe @BKPepe can comment on this? I've seen your commits every now and then and would add you as a committer but ideally someone with more packages.git insights does so.

@stangri
Copy link
Member Author

stangri commented Jul 23, 2021

PS. Even tho I've got the merge rights for luci pretty quickly, I've been asking for merge rights for packages for over a year now, can someone please explain to me what the process is and what should I do get things moving?

Who did you ask? Maybe @BKPepe can comment on this? I've seen your commits every now and then and would add you as a committer but ideally someone with more packages.git insights does so.

I've been asking in most of my PRs for packages. Is that not the procedure to request merge rights for this repo?

@neheb
Copy link
Contributor

neheb commented Jul 23, 2021

I got added either by @hnyman or @thess as I was handling a lot of package updates at the time.

I don't think there's any formal criteria.

@stangri
Copy link
Member Author

stangri commented Jul 23, 2021

Thanks, most of my older PRs were merged by @hnyman and @dibdot but lately it's mostly been @neheb and @BKPepe.

@aparcar
Copy link
Member

aparcar commented Jul 23, 2021

@neheb if you approve @stangri I'm happy to do the magic

@BKPepe
Copy link
Member

BKPepe commented Jul 23, 2021

I think we need to solve this #15257 before giving commit access to anyone.

@stangri
Copy link
Member Author

stangri commented Jul 24, 2021

I think we need to solve this #15257 before giving commit access to anyone.

I strongly believe that even if that's the rule, an exception can be made:

  1. There was no activity in the referred issue since March, there's no ETA when it's going to be implemented. I don't mind being limited to the packages I maintain, but that would delay me getting merge rights for an unforeseeable amount of time.
  2. This PR for example has been posted almost a week ago and as far as I understand, there's nothing preventing it from being merged, it's just a time issue for people who can merge it. Why not let me do it?
  3. My PRs have been accepted into packages repo and I've been a package maintainer since March 2017.
  4. I've been merging my own PRs for luci repo without any issues since May 2020 (with a few exceptions where I needed review for major changes).
  5. I can't find exact date when I first requested the merge rights for packages, but I believe it was about the same time I've requested merge rights for luci too, so before May 2020.

@neheb
Copy link
Contributor

neheb commented Jul 24, 2021

@neheb if you approve @stangri I'm happy to do the magic

Sure.

@aparcar
Copy link
Member

aparcar commented Jul 25, 2021

@stangri If you want other people to review your commits I kindly ask to split them into multiple smaller chunks rather than a single big change.

@stangri
Copy link
Member Author

stangri commented Jul 26, 2021

@stangri If you want other people to review your commits I kindly ask to split them into multiple smaller chunks rather than a single big change.

Would it be OK to submit PRs for smaller updates to master branch relatively frequently but then post combined larger PRs for stable every 3-4-6 months? I may have misunderstood @BKPepe but I believe he was opposed to frequent PRs to stable branches?

@aparcar
Copy link
Member

aparcar commented Jul 26, 2021

@stangri My approach for backporting is cherry-picking (git cherr-pick -x <commit id>) all commits that proved to work fine, here is an example: openwrt/luci#5210 I think @neheb point is to only apply maintenance updates aka point releases. Looking at Debian, you don't receive the latest Redis version in an older Debian release, you stick to whatever major release and only receive stability/security updates.

@champtar
Copy link
Member

@stangri some people want Debian stability, some want Arch / Fedora update pace, to make everyone happy make sure backports are fully run tested and you are good. Also do not backport big breaking changes except if it's really a blocker as people expect to just be able to upgrade with thinking about it ;)

@stangri
Copy link
Member Author

stangri commented Jul 26, 2021

@BKPepe @neheb @aparcar so the platform tests keep failing before trying to make this package. When I originally submitted the PR, only two platforms failed, now they all fail. Is it still safe to merge or should I keep re-running workflows daily until they all (or some) are green?

@neheb
Copy link
Contributor

neheb commented Jul 26, 2021

Dirty patches detected, please refresh and review the diff

This has nothing to do with tests.

@neheb
Copy link
Contributor

neheb commented Jul 26, 2021

Oh this has to do with openvswitch. I think this was fixed in master. Please rebase.

support for 21.02.0-rc2 and up
support for reloading a single interface on ifup/ifupdate
rename config file
updated shellcheck compatibility
remove obsolete create/remove_lock
interface processing optimizations to speed up reloads
drop dependency on curl in user scripts
uniform styling of functions

Signed-off-by: Stan Grishin <stangri@melmac.net>
@stangri
Copy link
Member Author

stangri commented Jul 27, 2021

Oh this has to do with openvswitch. I think this was fixed in master. Please rebase.

Thanks, seemed to help everything but x86_64, which also fails on 21.02 branch. Is it safe to merge then?

@neheb
Copy link
Contributor

neheb commented Jul 27, 2021

go ahead and merge. the test failure on x86 is unrelated.

@stangri stangri merged commit af44f17 into openwrt:master Jul 27, 2021
@stangri stangri deleted the master-vpn-policy-routing branch August 1, 2021 14:32
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.

None yet

5 participants