Resign Maintainership #435
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's get technical and review the patch :-).
I actually agree with the "-" part of the patch - having GP version of the plugin maintained by someone else than the upstream author is less than ideal. IMO it would be best having Matthew Brush at the "+" side of the patch if @codebrainz is willing to maintain it.
Otherwise I think it doesn't make sense to keep an unmaintained copy of a maintained plugin in GP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On 10 June 2016 at 21:36, Jiří Techet notifications@github.com wrote:
There is nothing to review. If a later patch adds a maintainer, fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But they don't have an upstream version from which they'd diverge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like having separate repo/issues/build system, etc, which is why I kept GeanyPy as external to Geany-Plugins. I don't think every plugin people write should be forced to be in Geany-Plugins. I don't mind formatting patches for changes to upstream so they can be applied to the fork in Geany-Plugins, but it's going to get harder and harder as the fork has diverged significantly from my repo (ex. the proxy plugin stuff, the geany_functions commit, etc).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, definitely not forced but let's say "strongly suggested" because having a plugin in GP is nice both for users and developers. I don't think there's a problem in having a separate development repo - I think even with this it could be quite easy to copy-over the sources from the development repo to GP before a release with a simple commit message like "updates from upstream" (something similar to what happens when Scintilla gets updated in Geany).
The question is what to do when someone makes a pull request to the plugin in GP - one option is to merge it in GP and apply it also upstream, the other is to reject the patch in GP and ask for submitting it upstream.
That's real pity it became a real fork - any chance to get the implementations merged so there's just one geanypy? Which one contains what - the GP one contains the proxy plugin stuff and yours not or the other way round?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless the GP maintainer intends to have a proper fork, IMO the proper way is to only accept Git-formatted patches from upstream, keeping the authorship and commit messages at least, if possible. Shouldn't allow changes downstream which are not first applied upstream. Something like #440 would enforce this and makes it easier to keep downstream tracking (whatever stable) upstream they want.
I don't think it would be really hard to bring them inline, but it would be better to do it after something like #440 so it can't happen again, IMO.
Yeah, the main repo doesn't have the proxy plugins stuff merged, mostly because a) it got rebased after I started reviewing/testing b) I wanted to retain compatibility so everybody's plugins didn't break and c) after I gave in and gave @kugel- the OK to merge it, I think he lost interest since it was merged in the fork already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
[Edit: it was actually my understanding when the proxy stuff was committed that the compatibility problems had been fixed, but it since appears not. This has (correctly IMHO) delayed committing it upstream]