4

Why so many open pull requests?

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.

Tyler Mandry's avatar
125
Tyler Mandry
asked 2013-01-10 16:34:50 -0500
edit flag offensive 0 remove flag close merge delete

Comments

add a comment see more comments

3 Answers

4

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.

Evgeny's avatar
13.2k
Evgeny
answered 2013-01-10 21:46:24 -0500, updated 2013-04-22 17:40:02 -0500
edit flag offensive 0 remove flag delete link

Comments

2

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's avatar Tyler Mandry (2013-01-11 00:17:03 -0500) edit
1

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's avatar bobbydavid (2013-04-22 16:52:20 -0500) edit

Some of those pull requests will probably be closed.

Evgeny's avatar Evgeny (2013-04-22 17:31:01 -0500) 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's avatar bobbydavid (2013-04-24 08:29:22 -0500) edit
add a comment see more comments
3

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's avatar
31
Felix Schwarz
answered 2013-01-15 12:57:39 -0500
edit flag offensive 0 remove flag delete link

Comments

add a comment see more comments
1
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. :)

pajju's avatar
565
pajju
answered 2013-04-23 10:31:59 -0500, updated 2013-04-23 10:34:37 -0500
edit flag offensive 0 remove flag delete link

Comments

add a comment see more comments