-
Notifications
You must be signed in to change notification settings - Fork 380
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
Merging the openwrt-routing/packages with openwrt/packages? #184
Comments
Seems reasonable to me to merge this within openwrt/packages. AFAIR this project started earlier than when we decided to move the packages maintenance to Github, so no objection from me. |
Hello @jow- I have a proposal, can we set up the repositories in a way that is never possible to push directly but it is always necessary a pull request ? In this way we know that each change had at least 1 other person reviewing it. |
@zioproto you can protect branches and enable require status checks. it's not the same, but nearby. |
No objections from my side |
sure, go ahead |
i'am against it. it's easier for me to follow in this special field and i dont want to read all the messages from packages. |
I see the point and for me both strategies are fine. Could it mean less flexibility when interested in bleeding-edge routing /axel On 04.05.2016 19:21, Jo-Philipp Wich wrote:
|
Hi –
+1 Cheers, Viral meme of radical freedom The fact that you talk in your head doesn't mean that you think. The best way to lose control over yourself is trying to control yourself. Most people experience themselves as a voice in their head, telling them |
any progress on that? |
So far (using members listed by github): +1 jow, zioproto, mwarning, axn, elektra42, ffainelli Give it a 2w timeout for the above to reply, no answer is counted as no opinion/blank. Seems reasonable and fair? Missing: |
Hi @jow- et al, I am in the boat with @zioproto - as long as maintainers can require patch peer-reviews for specific packages I am fine with merging. We already had the case where a well-intended batman-adv patch pushed by a non-maintainer broke our package while we maintainers were unaware of said change until people complained about the broken package. The protected branches mentioned by @lynxis does not sound like a solution to this problem. All packages live in subdirectories and not branches. Anybody else with a suggestion ? @diizzyy: Not sure why @zioproto is counted as 'yes' since the problem he raised remains unsolved. Cheers! |
@lindermarek: I think it's going to be hard to implement this as some packages are very use-case specific but I do agree there should be a pull request with some timeout for reviewing however for major changes. I don't see a need for minor changes however. |
@diizzyy: That differentiation is very fuzzy. What constitutes a major changes / a minor change ? In our case, the change was considered minor and broke the package nonetheless .. |
@lindnermarek That said, we should always require a run-time test. I don't think there's a simple fix for this, reasonable would be that maintainer(s) should be able to commit directly while "external" pull requests should be reviewed within a reasonable time otherwise a commit should be allowed to step in and commit if requirements are fulfilled and it wont affect other packages |
Recently the package smcroute was moved from this repo to openwrt/packages. The advantages what I can see are that openwrt/packages has CircleCI, so the packages are compile tested against ath79 target and it has DCO check (Signed-off-by commits). It's not a big deal to have it here as well. Let's look, for example, at #215 and we can think why the pull request wasn't tightened to the end. However, I think in openwrt/packages there's a lot of activity. This situation can be improved here if there will be more people with commit access or more people, who will maintain this repo. |
Still, It would be nice to have a repository that has relaxed rules (e.g. for less polished packages). |
I presume the goal is to have as little breakage as possible so another other option would be to detach the routing repo if people don't feel they want to raise the bar slightly. |
@diizzyy as the vote is already in favor of merging. How about doing that, and create another repository in the long run. But this decision is not part of this ticket. |
Following the discussion at openwrt-routing/packages there is a majority in favor to merge the two repositories[0]. The reasonable doubt of @bittorf[1] (notification bloat) is solvable by using a CODEOWNERS[2] file, allowing individual notifications on specific folders. This eventually unifies things belonging together, adds more eyes to the routing packages, introduces automatic pull requests testing and even removes a line from feeds.conf! [0]: openwrt/routing#184 (comment) [1]: openwrt/routing#184 (comment) [2]: https://help.github.com/en/articles/about-code-owners Signed-off-by: Paul Spooren <[email protected]>
I created a pull requests, basically moving all this to openwrt/packages/net, see here openwrt/packages#9399 We could start using a codeowners file to define responsibilities per package. |
It will work just for people, which has commit access. |
I don't see a problem giving the maintainers of routing access to packages, or would that be a problem? |
@aparcar But it won't be available to non-maintainers, so those who were watching the whole openwrt-routing would now have to watch the whole openwrt/packages? |
@adrianschmutzler those would likely only watch their own packages. |
Hi, it looks like this is stuck again. Unfortunately, almost nothing has happened since then. There have been two attempts: The latter (from me) was meant as a proof-of-concept for merging with history, which has been rejected as stated earlier. Since I'm not a package maintainer, and my contribution was limited to the history stuff, I will close that one again. Nevertheless, the terms should be clear now: |
It would be good to reach a decision (one way or the other) before the next 20.XX release. If I sum up the discussion so far: General proposalMove all packages from openwrt-routing/packages into openwrt/packages Arguments in favour
Arguments against
Remaining questionsHow to ensure that openwrt-routing maintainers are kept in the loopThis touches both on following activity and ensuring proper patch review. Codeowners files can be used, they work with both Github and Gitlab. Maintainers receive a special notification about their packages, and pull requests can be configured to require the approval of the maintainer. It seems it needs all maintainers to have commit rights. Commit rightsShould all current members of openwrt-routing obtain commit rights on openwrt/packages? How to import packagesThe current approach is to move packages individually, improving them in the process. Pro: it improves package quality and consistency. Cons: it takes time and efforts. This approach is preferred by openwrt/packages people, see openwrt/packages#9399 The alternative is to bulk-import all packages as they are currently in openwrt-routing. Another issue is whether to keep the history of commits: the current approach is to discard history, see openwrt/packages#9408 |
From my end, I also think that the activity on openwrt/packages is really too high to follow. I am subscribed to email notifications for it but I can't keep up with everything. The advantage of openwrt-routing is that it's a small group of people that (mostly) know each other, so it's easier to work together and trust each other. If there is a process to make sure that maintainers are made aware of changes and can review patches, then I'm ok with the merge. Otherwise I think it should stay separate, unless there are stronger arguments in favour of the merge. |
Based on feedback from IRC, I have updated the summary (about codeowners files, and importing packages individually) |
|
Looks to me like this will stay separate. Maybe just get CI running and be happy? |
There is still some discussion at openwrt/packages#12977 to fix the notification issue. Please don't close this just yet. |
Can we maybe just adopt the |
Hi, it's been some time and I'd like to move this at least over to the openwrt organization so all core members have access. GitHub allows to transfer ownership and renames with automatic forwarding, buildbots internally use the mirror on git.openwrt.org anyway. After the move I'd make sure that a group called routing-maintainers has commit access to the repository and I add all current commiters of openwrt-routing/packages to this group. Any objections? |
What are all the things which need updating / changing in order to not break after this transition ? URLs, notifications, git repositories, permissions, etc ? A comprehensive list would be appreciated to allow us to catch most of the issues beforehand. |
GitHub automagically redirects requests meaning we don't have to update anything during the migration, pulling from the old URL will still work. From my understanding the migration also transfers stars and notification settings. Long term the mirror script at git.openwrt.org should be updated. Apart from that the openwrt.git/README.md needs an update, but that's minor. |
Hi @openwrt-routing/owners
for a long time both the openwrt-routing and openwrt packages organizations are working in parallel and in many cases are maintained by the same persons.
For the sake of simplicity I'd like to suggest to merge both organizations and move the packages from here into openwrt/packages.
This issue is just for starting a discussion and maybe reaching a consensus, so please voice your opinion here.
The text was updated successfully, but these errors were encountered: