Why so many open pull requests?

bugs
development
github
retag add tags

It seems that the Pull Request page has some important fixes, including an XSS vulnerability fix, that have been ignored. Why is that? Was the vulnerability fixed elsewhere? (If so, someone should close the request.) I thought anyone would be able to contribute to AskBot, and it seems that fixes like that ought to be merged in quickly. This is an actual concern I have over adopting AskBot.

Your work has been excellent so far, I just think some more transparency would make it a killer project.

125

Tyler Mandry

asked 2013-01-10 16:34:50 -0600

edit flag offensive 0 remove flag close merge delete

Comments

add a comment see more comments

3 Answers

Sort by

oldest

newest

most voted

Tyler you are right to point this out, we should pay more attention to that. We will try to merge some requests soon.

A common problem with the pull requests is a lack of test cases, which makes it more difficult to integrate them.

edit 2 Roughly, we apply the following rules to accepting the pull requests:

Obvious bug fixes are accepted instantly
Translation contributions will no longer be accepted via github - please use [Transifex].(https://www.transifex.com/projects/p/askbot/)
If a pull request contains a new feature - I cannot guarantee merging without accepting the feature itself - regardless of the implementation. Please, let's discuss new features first.
More than one feature per pull request makes it less likely that for us to merge, which means - please use one topic branch per feature - forked directly from the Askbot's master branch.
New features must be implemented to decent quality and must be covered by test cases.
All existing test cases must pass in the pull request.

edit 1: it is true - we do not have any organization for the development, neither a roadmap, nor a process. I agree that it makes it more difficult for others to contribute.

13.2k

Evgeny

answered 2013-01-10 21:46:24 -0600, updated 2013-04-22 17:40:02 -0600

edit flag offensive 0 remove flag delete link

Comments

Thanks for your excellent answer. Maybe you could make a "Contributing" section in the readme that reminds people to add tests, etc.? In any case, you should let them know about the problem so they can address it :). Thanks! Hope to be contributing soon myself.

Tyler Mandry (2013-01-11 00:17:03 -0600) edit

I would also like to contribute, and I'm running into a similar problem. Please consider setting some time aside to "clean house" on the pull requests?

bobbydavid (2013-04-22 16:52:20 -0600) edit

Some of those pull requests will probably be closed.

Evgeny (2013-04-22 17:31:01 -0600) edit

Here is one reasonable course of action: unless a pull request is fresh and usable, reject it with a message to the contributer saying "sorry i am rejecting because of overload, if this is still a valid contribution, please update and re-submit"

bobbydavid (2013-04-24 08:29:22 -0600) edit

add a comment see more comments

In addition what Evgeny said I'd like to add that at least some of the pull requests look 'invalid' to me. At least one is clearly superseeded by an updated pull request. Another one just replaces 'karma' with 'reputation' without justification or backwards compatibility.

I suspect at least 1/3 of the pull requests is invalid but still a lot of developer time wasted. Probably you could get more useful contributions if you (Evgeny) had the time to share you thoughts/critic on the pull requests.

It might also help to have a publicly available continuous testing server (prominently displayed) so developers will be more aware of the testing requirement.

Felix Schwarz

answered 2013-01-15 12:57:39 -0600

edit flag offensive 0 remove flag delete link

Comments

add a comment see more comments

The problem here is:

Lot of pull requests are coming for New features.

And merging all such new-features with the core branch should be done slowly with the consensus from existing communities and growing needs. Lets go slow from here, and don't break things!

We are not Rails! :D

We need to have a clear developer documentation of How to contribute for askbot

Include coding style, test cases to write, better bug-tracker, Trello Boards or even better project management tools etc. This has to be done very soon.

Regarding passing those test cases:

Lets create a separate test case folder? With test scripts inside?

Well, most developers don't know the core Askbot code, they would have written test cases for their own code submissions, but testing with Askbot test cases is something we should not expect from them.

The core contributors and askbot team should be assigned to test all the global test cases when merging with the core and report back bugs to the original authors.

Open for comments. :)