PR checklist proposal
140 views
Skip to first unread message
Jason Moore
Mar 13, 2015, 4:41:34 PM3/13/15
to sy...@googlegroups.com
How would people feel if a checklist like this was add to each new PR and it was part of our policy?
- Is it mergeable?
- Did it pass the tests?
-
If it introduces new functionality is it tested? Do public methods/classes have docstrings? Did you add an explanation to the online documentation?
-
Is it well formatted? (list ways to run linters)
- Is it documented in the Release Notes?
I submitted a PR to another project a while back and they had this system in place. It seemed like a good idea to me.
Aaditya Nair
Mar 13, 2015, 5:26:44 PM3/13/15
to sy...@googlegroups.com
Personally, I think that this system is way too formal. Most of them are documented in the SymPy Contribution Guide.
Also, new contributors may not have enough knowledge to actually fill out these checklists on their own. They will need a mentor to guide them through the process.
Now, through the interaction the mentor has a general idea if the above criterion is being fulfilled.
Although I believe that this checklist should be emphasized in the Contribution Guide.
Aaditya M Nair.
Jason Moore
Mar 13, 2015, 5:54:11 PM3/13/15
to sy...@googlegroups.com
Responses below.
On Fri, Mar 13, 2015 at 2:26 PM, Aaditya Nair <aaditya...@gmail.com> wrote:
Personally, I think that this system is way too formal.
Sounds like you may think formality is a negative thing. If so can you explain why?
Most of them are documented in the SymPy Contribution Guide.
If we have these guidelines in the contributor's guide, then how does it hurt to add it them to each PR?
Also, new contributors may not have enough knowledge to actually fill out these checklists on their own. They will need a mentor to guide them through the process.
We already mentor new contributors and would not expect them to add this or even fill it out. But the PR reviewer (i.e. mentor) would be responsible in explaining these things to a new contributor. In fact, if this is on the PR the new contributor is much more likely to be aware of them as opposed to being in the wiki somewhere.
Now, through the interaction the mentor has a general idea if the above criterion is being fulfilled.
PR reviewers +1 PRs all the time without all the contributor guidelines being met. This can also serve as a reminder to them.
Although I believe that this checklist should be emphasized in the Contribution Guide.Aaditya M Nair.
--
You received this message because you are subscribed to the Google Groups "sympy" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sympy+un...@googlegroups.com.
To post to this group, send email to sy...@googlegroups.com.
Visit this group at http://groups.google.com/group/sympy.
To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/c0bb0d0d-afb4-44aa-a85f-a50249d90fb5%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Joachim Durchholz
Mar 14, 2015, 3:10:09 AM3/14/15
to sy...@googlegroups.com
Can't talk for Aaditya, but here are my sentiments.
Please do not take that as being against the proposal, I'm just trying
to explain how his points are valid (but they are not the only points,
more on that in the answer to your starting post).
Am 13.03.2015 um 22:53 schrieb Jason Moore:
> On Fri, Mar 13, 2015 at 2:26 PM, Aaditya Nair <aaditya...@gmail.com>
> wrote:
>
>> Personally, I think that this system is way too formal.
>
> Sounds like you may think formality is a negative thing. If so can you
> explain why?
>
>> Most of them are documented in the SymPy Contribution Guide.
>
> If we have these guidelines in the contributor's guide, then how does it
> hurt to add it them to each PR?
Both aspects have the same kind of problem: They add to what a
contributor must observe.
So yes these things do hurt, and introducing them means they need to
offset that with gains.
>> Also, new contributors may not have enough knowledge to actually fill out
>> these checklists on their own. They will need a mentor to guide them
>> through the process.
>
> We already mentor new contributors
Only those who come through GSoC.
E.g. I never got a mentor. In fact I might have been deterred if being
mentored had been a prerequisite.
> and would not expect them to add this or
> even fill it out. But the PR reviewer (i.e. mentor) would be responsible in
> explaining these things to a new contributor.
I suspect the bigger problem is to get more reviewing done.
I'm feeling that pain myself. I'd like to do more reviewing, but
whenever I look at a PR, in 90+% I see that I have far too little
understanding of the code that's being modified or extended to say many
meaningful things.
Fixing the formalities is the easy part, but if we add a procedure for
these, this (usually) just means that the things that cannot be made
formal get forgotten because everybody is so occupied with getting the
formalities right.
That does not mean that formalities are useless or irrelevant. But they
need to be put into perspective, the informal aspects need to be spelled
out as well.
> In fact, if this is on the PR
> the new contributor is much more likely to be aware of them as opposed to
> being in the wiki somewhere.
True.
>> Now, through the interaction the mentor has a general idea if the above
>> criterion is being fulfilled.
>
> PR reviewers +1 PRs all the time without all the contributor guidelines
> being met. This can also serve as a reminder to them.
Good point.
I think a lot depends on how it is implemented, and since the proposal
is somewhat vague in that respect, people tend to fill in their previous
experiences with such things - and formal stuff often comes as a
replacement, not complement of informal stuff which is just as
important, so these experiences tend to be negative.
Please do not take that as being against the proposal, I'm just trying
to explain how his points are valid (but they are not the only points,
more on that in the answer to your starting post).
Am 13.03.2015 um 22:53 schrieb Jason Moore:
> On Fri, Mar 13, 2015 at 2:26 PM, Aaditya Nair <aaditya...@gmail.com>
> wrote:
>
>> Personally, I think that this system is way too formal.
>
> Sounds like you may think formality is a negative thing. If so can you
> explain why?
>
>> Most of them are documented in the SymPy Contribution Guide.
>
> If we have these guidelines in the contributor's guide, then how does it
> hurt to add it them to each PR?
contributor must observe.
So yes these things do hurt, and introducing them means they need to
offset that with gains.
>> Also, new contributors may not have enough knowledge to actually fill out
>> these checklists on their own. They will need a mentor to guide them
>> through the process.
>
> We already mentor new contributors
E.g. I never got a mentor. In fact I might have been deterred if being
mentored had been a prerequisite.
> and would not expect them to add this or
> even fill it out. But the PR reviewer (i.e. mentor) would be responsible in
> explaining these things to a new contributor.
I'm feeling that pain myself. I'd like to do more reviewing, but
whenever I look at a PR, in 90+% I see that I have far too little
understanding of the code that's being modified or extended to say many
meaningful things.
Fixing the formalities is the easy part, but if we add a procedure for
these, this (usually) just means that the things that cannot be made
formal get forgotten because everybody is so occupied with getting the
formalities right.
That does not mean that formalities are useless or irrelevant. But they
need to be put into perspective, the informal aspects need to be spelled
out as well.
> In fact, if this is on the PR
> the new contributor is much more likely to be aware of them as opposed to
> being in the wiki somewhere.
>> Now, through the interaction the mentor has a general idea if the above
>> criterion is being fulfilled.
>
> PR reviewers +1 PRs all the time without all the contributor guidelines
> being met. This can also serve as a reminder to them.
I think a lot depends on how it is implemented, and since the proposal
is somewhat vague in that respect, people tend to fill in their previous
experiences with such things - and formal stuff often comes as a
replacement, not complement of informal stuff which is just as
important, so these experiences tend to be negative.
Joachim Durchholz
Mar 14, 2015, 3:24:53 AM3/14/15
to sy...@googlegroups.com
Am 13.03.2015 um 21:41 schrieb Jason Moore:
> How would people feel if a checklist like this was add to each new PR and
> it was part of our policy?
This depends on where in the process the checklist is displayed, and how
> How would people feel if a checklist like this was add to each new PR and
> it was part of our policy?
much it urges people to work on the checklist and what other, possibly
equally important tasks get pushed into the background by the
requirement to do the checklist.
> - Is it mergeable?
> - Did it pass the tests?
This is already taken care for in the status line of the github
discussion: People see whether a PR is mergeable, whether the tests
pass, and they're getting scary red markers if that doesn't work.
> - If it introduces new functionality is it tested?
This leaves open how to determine that the test suite is complete.
> Do public methods/classes have docstrings?
For example, most docstrings do not list preconditions and
postconditions. Even in those cases where they aren't trivially inferred
from other hints.
Also, I'm often having trouble to determine how the code relates to the
math it is implementing.
> Did you add an explanation to the online
> documentation?
> - Is it well formatted? (list ways to run linters)
We already have a PEP8 requirement, and test_code_quality takes care of
that.
If any improvements in that area are required, they should go into
test_code_quality.
> - Is it documented in the Release Notes?
I think that's taken care of by whoever prepares the release notes.
(I may be wrong.)
> I submitted a PR to another project a while back and they had this system
> in place. It seemed like a good idea to me.
these questions need to be answered:
- What problems did it solve there?
- Do these problems need solving in SymPy?
- What other problems need solving in SymPy? Would these solutions
complement, replace or contradict a PR checklist?
Note I'm not opposed to the idea as such.
It's just an idea though, and not yet mature enough to know whether it
will be a help or a hindrance. (IMHO)
Francesco Bonazzi
Mar 14, 2015, 7:31:28 AM3/14/15
to sy...@googlegroups.com
On Saturday, March 14, 2015 at 8:10:09 AM UTC+1, Joachim Durchholz wrote:
I'm feeling that pain myself. I'd like to do more reviewing, but
whenever I look at a PR, in 90+% I see that I have far too little
understanding of the code that's being modified or extended to say many
meaningful things.
May there should be many more comments in the code. For instance, it is a good practice to put a comment in front of every for- and while-loop to describe what it does, if that's not immediately clear from the code. It would also be nice to have comments explaining the expected types of function parameters, return types and possible exceptions raised.
Another good habit would be to put a summary of the PR discussion in the source code, so that future reviewers will be able to easily understand what the code was meant and what it does, without any need to look for past PR discussions.
Jason Moore
Mar 14, 2015, 12:18:07 PM3/14/15
to sy...@googlegroups.com
On Sat, Mar 14, 2015 at 12:10 AM, Joachim Durchholz <j...@durchholz.org> wrote:
Can't talk for Aaditya, but here are my sentiments.
Please do not take that as being against the proposal, I'm just trying to explain how his points are valid (but they are not the only points, more on that in the answer to your starting post).
Am 13.03.2015 um 22:53 schrieb Jason Moore:
On Fri, Mar 13, 2015 at 2:26 PM, Aaditya Nair <aaditya...@gmail.com>
wrote:
Personally, I think that this system is way too formal.
Sounds like you may think formality is a negative thing. If so can you
explain why?
Most of them are documented in the SymPy Contribution Guide.
If we have these guidelines in the contributor's guide, then how does it
hurt to add it them to each PR?
Both aspects have the same kind of problem: They add to what a contributor must observe.
So yes these things do hurt, and introducing them means they need to offset that with gains.
This doesn't seem like it adds anything to what a contributor must observe because we already have these guidelines. It simply makes it more visible and introduces a process to ensure we think about them at each PR.
Also, new contributors may not have enough knowledge to actually fill out
these checklists on their own. They will need a mentor to guide them
through the process.
We already mentor new contributors
Only those who come through GSoC.
E.g. I never got a mentor. In fact I might have been deterred if being mentored had been a prerequisite.
I meant this in the sense that every PR reviewer is "mentoring". If the PR reviewer says "please write a unit test", then they are effectively "mentoring". This checklist would help us not have to list the common things so much.
> and would not expect them to add this or
even fill it out. But the PR reviewer (i.e. mentor) would be responsible in
explaining these things to a new contributor.
I suspect the bigger problem is to get more reviewing done.
I'm feeling that pain myself. I'd like to do more reviewing, but whenever I look at a PR, in 90+% I see that I have far too little understanding of the code that's being modified or extended to say many meaningful things.
Fixing the formalities is the easy part, but if we add a procedure for these, this (usually) just means that the things that cannot be made formal get forgotten because everybody is so occupied with getting the formalities right.
That does not mean that formalities are useless or irrelevant. But they need to be put into perspective, the informal aspects need to be spelled out as well.
I definitely want to increase reviewing and not add frivolous formalities.
If we have this checklist, new reviewers may be more willing to review at least for the things in the checklist because it is very obvious what the reviewer should be looking for. This could actually help ease people into reviewing.
If we have this checklist, new reviewers may be more willing to review at least for the things in the checklist because it is very obvious what the reviewer should be looking for. This could actually help ease people into reviewing.
People forgetting to actually read the code and only checking the boxes code be bad. The only way to see if this happens would be to try out the system. The project I grabbed this idea from didn't seem to have that problem.
I don't see this as a frivolous formality. I think we will see gains in code quality and reduced effort by PR reviewers because we often have to write these things in the PR anyways.
> In fact, if this is on the PR
the new contributor is much more likely to be aware of them as opposed to
being in the wiki somewhere.
True.
Now, through the interaction the mentor has a general idea if the above
criterion is being fulfilled.
PR reviewers +1 PRs all the time without all the contributor guidelines
being met. This can also serve as a reminder to them.
Good point.
I think a lot depends on how it is implemented, and since the proposal is somewhat vague in that respect, people tend to fill in their previous experiences with such things - and formal stuff often comes as a replacement, not complement of informal stuff which is just as important, so these experiences tend to be negative.
--
You received this message because you are subscribed to the Google Groups "sympy" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sympy+unsubscribe@googlegroups.com.
To post to this group, send email to sy...@googlegroups.com.
Visit this group at http://groups.google.com/group/sympy.
To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/5503DECA.2000001%40durchholz.org.
Jason Moore
Mar 14, 2015, 12:26:06 PM3/14/15
to sy...@googlegroups.com
More...
On Sat, Mar 14, 2015 at 12:24 AM, Joachim Durchholz <j...@durchholz.org> wrote:
Am 13.03.2015 um 21:41 schrieb Jason Moore:
How would people feel if a checklist like this was add to each new PR and
it was part of our policy?
This depends on where in the process the checklist is displayed, and how much it urges people to work on the checklist and what other, possibly equally important tasks get pushed into the background by the requirement to do the checklist.
I'd like to propose to try it on some PRs to see how it goes. That's the only way to find out.
- Is it mergeable?
- Did it pass the tests?
This is already taken care for in the status line of the github discussion: People see whether a PR is mergeable, whether the tests pass, and they're getting scary red markers if that doesn't work.
Travis only reports if the tests pass on one flavor of linux. Asking them to run the tests locally gives us that much more coverage on more operating systems.
The mergeable question is redundant.
- If it introduces new functionality is it tested?
This leaves open how to determine that the test suite is complete.
That is still up to the reviewer. We get lots of PRs that initially do not have tests. This is a simple reminder to write something and it is likely if they write something for a test they may write a suitable suite.
> Do public methods/classes have docstrings?
This leaves open what should be in a docstring.
For example, most docstrings do not list preconditions and postconditions. Even in those cases where they aren't trivially inferred from other hints.
Also, I'm often having trouble to determine how the code relates to the math it is implementing.
We can add a link to our docstring guidelines. Once again this check is simply to ensure there is a docstring, not how good it is. That's the reviewers call.
> Did you add an explanation to the online
> documentation?
Isn't that generated from the docstrings?
Yes and no. People add features all of the time with only docstrings. This is great but many people learn to use the functionality best from the prose style docs. This would encourage the person to have a look at the online docs and see if their feature is explained anywhere.
- Is it well formatted? (list ways to run linters)
We already have a PEP8 requirement, and test_code_quality takes care of that.
If any improvements in that area are required, they should go into test_code_quality.
Yes, since we don't run PEP8 on travis, this will show how to run the code quality test. It is a reminder for people to do it locally.
- Is it documented in the Release Notes?
I think that's taken care of by whoever prepares the release notes.
(I may be wrong.)
Ideally, The Release Notes should be updated after each PR by someone involved the PR. This rarely happens and we have poor release notes. Check out pandas release notes for example. Many of the other major python packages kick our butt in release notes.
I submitted a PR to another project a while back and they had this system
in place. It seemed like a good idea to me.
To determine whether that experience can and should be transferred, these questions need to be answered:
- What problems did it solve there?
- Do these problems need solving in SymPy?
- What other problems need solving in SymPy? Would these solutions complement, replace or contradict a PR checklist?
How about we try it out on some PRs and see if it is a burden and whether it helps.
Note I'm not opposed to the idea as such.
It's just an idea though, and not yet mature enough to know whether it will be a help or a hindrance. (IMHO)
--
You received this message because you are subscribed to the Google Groups "sympy" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sympy+unsubscribe@googlegroups.com.
To post to this group, send email to sy...@googlegroups.com.
Visit this group at http://groups.google.com/group/sympy.
To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/5503E240.5040901%40durchholz.org.
Matthew Brett
Mar 14, 2015, 2:16:46 PM3/14/15
to sympy
Hi,
On Sat, Mar 14, 2015 at 9:17 AM, Jason Moore <moore...@gmail.com> wrote:
[snip]
[snip]
>> I suspect the bigger problem is to get more reviewing done.
>> I'm feeling that pain myself. I'd like to do more reviewing, but whenever
>> I look at a PR, in 90+% I see that I have far too little understanding of
>> the code that's being modified or extended to say many meaningful things.
>> Fixing the formalities is the easy part, but if we add a procedure for
>> these, this (usually) just means that the things that cannot be made formal
>> get forgotten because everybody is so occupied with getting the formalities
>> right.
>> That does not mean that formalities are useless or irrelevant. But they
>> need to be put into perspective, the informal aspects need to be spelled out
>> as well.
>
>
> I definitely want to increase reviewing and not add frivolous formalities.
>
> If we have this checklist, new reviewers may be more willing to review at
> least for the things in the checklist because it is very obvious what the
> reviewer should be looking for. This could actually help ease people into
> reviewing.
As an occasional contributor, I would find it very helpful to have
this checklist at the top.
I find that when I am thinking of contributing a PR to a project I
don't know well, a large barrier is the general feeling I don't know
what the rules are, for doing something that's useful and acceptable
to the community of the project. Having these set out right at the
top of the PR goes a long way to lowering that barrier - it changes a
vague feeling of 'not sure' to a specific set of tasks, which is much
easier to deal with...
Cheers,
Matthew
On Sat, Mar 14, 2015 at 9:17 AM, Jason Moore <moore...@gmail.com> wrote:
[snip]
[snip]
>> I suspect the bigger problem is to get more reviewing done.
>> I'm feeling that pain myself. I'd like to do more reviewing, but whenever
>> I look at a PR, in 90+% I see that I have far too little understanding of
>> the code that's being modified or extended to say many meaningful things.
>> Fixing the formalities is the easy part, but if we add a procedure for
>> these, this (usually) just means that the things that cannot be made formal
>> get forgotten because everybody is so occupied with getting the formalities
>> right.
>> That does not mean that formalities are useless or irrelevant. But they
>> need to be put into perspective, the informal aspects need to be spelled out
>> as well.
>
>
> I definitely want to increase reviewing and not add frivolous formalities.
>
> If we have this checklist, new reviewers may be more willing to review at
> least for the things in the checklist because it is very obvious what the
> reviewer should be looking for. This could actually help ease people into
> reviewing.
this checklist at the top.
I find that when I am thinking of contributing a PR to a project I
don't know well, a large barrier is the general feeling I don't know
what the rules are, for doing something that's useful and acceptable
to the community of the project. Having these set out right at the
top of the PR goes a long way to lowering that barrier - it changes a
vague feeling of 'not sure' to a specific set of tasks, which is much
easier to deal with...
Cheers,
Matthew
Joachim Durchholz
Mar 15, 2015, 5:34:29 AM3/15/15
to sy...@googlegroups.com
One some thinking and some reading of responses, my current thinking is
this:
1) This initially looked like a proposal for frivolous formality (nice
terminology btw), and I guess that's what ticked off Aaditya and me.
Fortunately that's off the table for good (at least for me).
2) What's still somewhat unclear is the goals. Or maybe I'm missing
arguments that link SymPy's project goals to the proposal.
3) Actually I think you're trying to address shortcomings in SymPy's
overall organization, at least in part.
I'd like to see what problems you mean to address, for various reasons:
- An experiment without a goal is an experiment where you never know
when it's done, or whether it was successful or not, or whether another
experiment is require.
- It remains unclear what other organisational issues SymPy needs to
have addressed. We might be experimenting with a symptom while the real
issue is elsewhere (I do have my ideas, probably like most anybody who
has an interest in such things; I'm currently halfway through
http://producingoss.com/ and can only recommend it, both for it advice
and for identifying discussion-worthy aspects in any open source
organization - it's one of those texts that are helpful even where one
disagrees).
4) If the experiment is done, do not name it "checklist" - that evokes
images of a strict list of things to be checked before the commit can go in.
Maybe something like "aspects to look at". Or "review guidelines".
5) What's missing is a list of actual review guidelines.
I think the usefulness of the proposal is in direct proportion to the
quality of the guidelines - if the guidelines suck, then having them
included in the PR process will suck; if the guidelines draw attention
to the important points, they can be immensely helpful.
6) A technicality: I'm not sure how such things can be automatically
included in a PR on github.
Also, I remember being only vaguely aware of any review guidelines;
maybe they should be listed in the contributor's guide.
I may be misremembering stuff though.
I hope this helps.
Regards,
Jo
this:
1) This initially looked like a proposal for frivolous formality (nice
terminology btw), and I guess that's what ticked off Aaditya and me.
Fortunately that's off the table for good (at least for me).
2) What's still somewhat unclear is the goals. Or maybe I'm missing
arguments that link SymPy's project goals to the proposal.
3) Actually I think you're trying to address shortcomings in SymPy's
overall organization, at least in part.
I'd like to see what problems you mean to address, for various reasons:
- An experiment without a goal is an experiment where you never know
when it's done, or whether it was successful or not, or whether another
experiment is require.
- It remains unclear what other organisational issues SymPy needs to
have addressed. We might be experimenting with a symptom while the real
issue is elsewhere (I do have my ideas, probably like most anybody who
has an interest in such things; I'm currently halfway through
http://producingoss.com/ and can only recommend it, both for it advice
and for identifying discussion-worthy aspects in any open source
organization - it's one of those texts that are helpful even where one
disagrees).
4) If the experiment is done, do not name it "checklist" - that evokes
images of a strict list of things to be checked before the commit can go in.
Maybe something like "aspects to look at". Or "review guidelines".
5) What's missing is a list of actual review guidelines.
I think the usefulness of the proposal is in direct proportion to the
quality of the guidelines - if the guidelines suck, then having them
included in the PR process will suck; if the guidelines draw attention
to the important points, they can be immensely helpful.
6) A technicality: I'm not sure how such things can be automatically
included in a PR on github.
Also, I remember being only vaguely aware of any review guidelines;
maybe they should be listed in the contributor's guide.
I may be misremembering stuff though.
I hope this helps.
Regards,
Jo
Aaron Meurer
Mar 15, 2015, 1:57:20 PM3/15/15
to sy...@googlegroups.com
Jason, feel free to give this a try. As you note, the only way to see
if it will work is to try it. My main concerns with it are
- It might create unnecessary friction, raising the barrier for
contribution, especially new contribution.
- People will want to add more and more things to the checklist,
making it even harder to pass. And some things might even be
unnecessary in the first place. Francesco's ideas, both of which I
think are bad (sorry Francesco), are an inadvertent demonstration of
this. Plus the checklist itself would become a huge bikeshed topic.
I suppose the main thing I'm unsure about is a formal checklist.
Informal checklists, though, I like, and it's something many reviewers
already do, and I would encourage more of. These tend to include the
sorts of things you noted, but are also many things that are specific
to the PR.
Aaron Meurer
if it will work is to try it. My main concerns with it are
- It might create unnecessary friction, raising the barrier for
contribution, especially new contribution.
- People will want to add more and more things to the checklist,
making it even harder to pass. And some things might even be
unnecessary in the first place. Francesco's ideas, both of which I
think are bad (sorry Francesco), are an inadvertent demonstration of
this. Plus the checklist itself would become a huge bikeshed topic.
I suppose the main thing I'm unsure about is a formal checklist.
Informal checklists, though, I like, and it's something many reviewers
already do, and I would encourage more of. These tend to include the
sorts of things you noted, but are also many things that are specific
to the PR.
Aaron Meurer
> --
> You received this message because you are subscribed to the Google Groups
> "sympy" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to sympy+un...@googlegroups.com.
> You received this message because you are subscribed to the Google Groups
> "sympy" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> To post to this group, send email to sy...@googlegroups.com.
> Visit this group at http://groups.google.com/group/sympy.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/sympy/5505521F.5030104%40durchholz.org.
> Visit this group at http://groups.google.com/group/sympy.
> To view this discussion on the web visit
Ondřej Čertík
Mar 16, 2015, 1:07:21 PM3/16/15
to sympy
On Sun, Mar 15, 2015 at 11:56 AM, Aaron Meurer <asme...@gmail.com> wrote:
> Jason, feel free to give this a try. As you note, the only way to see
> if it will work is to try it. My main concerns with it are
>
> - It might create unnecessary friction, raising the barrier for
> contribution, especially new contribution.
>
> - People will want to add more and more things to the checklist,
> making it even harder to pass. And some things might even be
> unnecessary in the first place. Francesco's ideas, both of which I
> think are bad (sorry Francesco), are an inadvertent demonstration of
> this. Plus the checklist itself would become a huge bikeshed topic.
>
> I suppose the main thing I'm unsure about is a formal checklist.
> Informal checklists, though, I like, and it's something many reviewers
> already do, and I would encourage more of. These tend to include the
> sorts of things you noted, but are also many things that are specific
> to the PR.
These are exactly my thoughts as well.
> Jason, feel free to give this a try. As you note, the only way to see
> if it will work is to try it. My main concerns with it are
>
> - It might create unnecessary friction, raising the barrier for
> contribution, especially new contribution.
>
> - People will want to add more and more things to the checklist,
> making it even harder to pass. And some things might even be
> unnecessary in the first place. Francesco's ideas, both of which I
> think are bad (sorry Francesco), are an inadvertent demonstration of
> this. Plus the checklist itself would become a huge bikeshed topic.
>
> I suppose the main thing I'm unsure about is a formal checklist.
> Informal checklists, though, I like, and it's something many reviewers
> already do, and I would encourage more of. These tend to include the
> sorts of things you noted, but are also many things that are specific
> to the PR.
Ondrej
Jason Moore
Mar 22, 2015, 9:56:41 PM3/22/15
to sy...@googlegroups.com
We are going to try this out in PyDy and see how it goes. I'll bring it back up in SymPy if our experiment goes well. Thanks for the feedback.
https://github.com/pydy/pydy/pull/146
https://github.com/pydy/pydy/pull/146
--
You received this message because you are subscribed to the Google Groups "sympy" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sympy+un...@googlegroups.com.
To post to this group, send email to sy...@googlegroups.com.
Visit this group at http://groups.google.com/group/sympy.
To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CADDwiVDkXLv2GQ%2B68VdGJSJC%3DUof08hWi7SCaJCOM6pxxtgdDg%40mail.gmail.com.
Reply all
Reply to author
Forward
0 new messages