Date   
Re: Merge other developers PRs?

danielb987
 

Mee to.

Daniel

2020-08-05 01:29 skrev Dave Heap:

I support that.
Dave

On 5 Aug 2020, at 6:53 AM, Bob Jacobsen <@BobJacobsen> wrote:
We’re starting to see overlaps in the 41 outstanding PRs.
Although other aspects are not clear, it seems we do have a consensus that we can merge PRs with one or more positive non-author reviews, no outstanding comments, and required CI complete.
Unless somebody objects, late this evening I’ll start merging PRs that meet those criteria, from lowest numbered up.

Re: Merge other developers PRs?

Dave Heap
 

I support that.

Dave

On 5 Aug 2020, at 6:53 AM, Bob Jacobsen <@BobJacobsen> wrote:

We’re starting to see overlaps in the 41 outstanding PRs.

Although other aspects are not clear, it seems we do have a consensus that we can merge PRs with one or more positive non-author reviews, no outstanding comments, and required CI complete.

Unless somebody objects, late this evening I’ll start merging PRs that meet those criteria, from lowest numbered up.

Re: Merge other developers PRs?

Bob Jacobsen
 

We’re starting to see overlaps in the 41 outstanding PRs.

Although other aspects are not clear, it seems we do have a consensus that we can merge PRs with one or more positive non-author reviews, no outstanding comments, and required CI complete.

Unless somebody objects, late this evening I’ll start merging PRs that meet those criteria, from lowest numbered up.

Let me know if you’re prefer I not do this.

(And I’d appreciate somebody taking a look at #8869, #8879, #8883, #8887, and #8892)

Bob

Bob Jacobsen
@BobJacobsen

Re: Surveying users on use of help/web #WebHelpUpdate

JerryG
 

The results are in.  Almost 160 usable surveys (not counting developers who did the pre-test) were filed over a two week period ending 7/23 from over a dozen countries, (60%+ US, from 34 states).  I put my results document into file folder JerryG in this forum, and will distribute a similar version (without my intermediate work sheets) to the Users Group.  Please scan the almost 100 comments that people wrote in (several key ones summarized below).  Post to thread with your comments.

 

Here are a few highlights that I picked out:

90% of respondents have access to internet when working with JMRI.

 

76% use Help from a JMRI window at least some of the time.

81% access the web site for help at least some of the time.

62% ask questions in the user forum (which is interesting since you would likely have to be a user of the forum to even know about the survey - lots of lurkers).

Only 30% ask a friend for help!

 

90% find the help they need via Help/web at least some of the time but only 37% say “most of the time.”

33% find errors in help/web some of the time.

62% use the Table of Contents.

63% use the Index.

Only 34% use the Glossary.

 

29% are on 4.19x.

32% are on 4.20 (note that this was only 1-2 weeks after 4.20 came out).

Only 30% report updating to production release soon after it comes out.

37% report usually installing test releases. 

 

56% will report a problem to the user group at least some of the time

18% report a problem to the Issues List in Github

 

Collectively, they reported using over 20 other forums.

 

Some key write-in responses (edited, grouped, summarized):

Great software, appreciate all your work, "best community maintained s/w I have seen"  (more than a dozen kudos).

Have difficulty finding content, needs more cross references, etc.

Explanations not sufficiently geared to beginners (should be “like we were 5th graders”).

Need “getting started,” step-by-step, how the pieces fit together

Indicate what version the screen shots are from and that the text refers to (much is out of date or inconsistent).

Nice to have many ways to do essentially the same thing - but need comparison of pros/cons of each (e.g Logix vs scripts vs Routes, LRoutes vs Routes).

 

For the user group: Those who know nothing about a problem should refrain from saying or requesting info as if they do.

 

So I would say this was affirming of the value of help info, although pointing out the interest in simpler explanations and more “integrating” step-by-step pages (like the one Dave Sands just did for PanelPro).  The idea of labeling each page or picture with what release it refers to might help to avoid confusion as pages get out of date (while not exactly a solution to this issue,  we have just added a line to the standard Footer that provides a pointer to the page history).  So definitely some things to think about as we create/update help info.

 

I’ll post a version of this to the Users’ Forum in a couple of days.

 

Jerry

 

P.S.  Is there anything useful we can do with the names/emails of the people who provided them?  About 60.

Re: Find some consensus

Dave Sand
 

I think the following line needs to be changed:

- All CI tests have passed for the Pull Request

It probably should be "All required CI..."

Dave Sand

----- Original message -----
From: Bob Jacobsen <@BobJacobsen>
To: "jmri@jmri-developers.groups.io Notification" <jmri@jmri-developers.groups.io>
Subject: Re: [jmri-developers] Find some consensus
Date: Sunday, August 02, 2020 10:47 AM

I’ve added some proposed changes in PR pabender#17

https://github.com/pabender/JMRI/pull/17

Direct link to proposed changes:
https://github.com/pabender/JMRI/pull/17/files

Page as changed:
https://htmlpreview.github.io/?https://github.com/bobjacobsen/JMRI/PR8865-bis/help/en/html/doc/Technical/gitadmin.shtml

Bob

On Aug 1, 2020, at 7:59 PM, danielb987 <db123@...> wrote:

Please read #8865 and give your comments on it.

Update github administration page to reflect current discussion
https://github.com/JMRI/JMRI/pull/8865

Bob Jacobsen
@BobJacobsen

Re: Find some consensus

Bob Jacobsen
 

I’ve added some proposed changes in PR pabender#17

https://github.com/pabender/JMRI/pull/17

Direct link to proposed changes:
https://github.com/pabender/JMRI/pull/17/files

Page as changed:
https://htmlpreview.github.io/?https://github.com/bobjacobsen/JMRI/PR8865-bis/help/en/html/doc/Technical/gitadmin.shtml

Bob

On Aug 1, 2020, at 7:59 PM, danielb987 <db123@...> wrote:

Please read #8865 and give your comments on it.

Update github administration page to reflect current discussion
https://github.com/JMRI/JMRI/pull/8865

Bob Jacobsen
@BobJacobsen

Re: Find some consensus

John Baldwin
 

I am new to this group and Java programming but am a programmer with 45+ years of experience.

I would second Bob's comments.

request adding "nor increase the workload of developers nor is a incomplete change to structure or practice that will require future work by others." (Maybe that should be a bulleted list instead of a run-on sentence).
and

I would prefer an additional sentence: "Longer discussions should be moved the the jmri-users list to make sure people become aware of them"
On this second point, I would suggest that more controversial changes trigger a notification to all the contributors of the affected code/subject area and an additional waiting period of at least a couple of weeks to give knowledgeable contributors a chance to review if they are offline for a while.
Thanks,
John Baldwin on the Delmarva Peninsula

Re: Find some consensus

danielb987
 

All,

Please read #8865 and give your comments on it.

Update github administration page to reflect current discussion
https://github.com/JMRI/JMRI/pull/8865

Daniel

Re: jmri.util.JUnitUtil not found?

Pete Cressman
 

Hey, thanks. I'll save this for the next time it happens.

On Friday, July 31, 2020, 04:42:38 PM PDT, Steve_G <railrodder@...> wrote:


Ok. I had this problem a couple of days ago. I had to, outside of eclipse, do an ant realclean; mvn compile; mvn test-compile
Then open eclipse and right click on pom file, update project. Then I think I had to do rebuild project. Sorry if I'm a bit hazy, didn't think it was a wide spread problem.
Steve G.

Re: jmri.util.JUnitUtil not found?

Steve_G
 

Ok. I had this problem a couple of days ago. I had to, outside of eclipse, do an ant realclean; mvn compile; mvn test-compile
Then open eclipse and right click on pom file, update project. Then I think I had to do rebuild project. Sorry if I'm a bit hazy, didn't think it was a wide spread problem.
Steve G.

Re: jmri.util.JUnitUtil not found?

Pete Cressman
 

The  "C:\Git\JMRI\temp” directory exits and looks OK.

I did a commit and push, even though i couldn't fully test. Then, remarkably, the "cannot resolve" error disappeared and the tests ran - without a recompile. (from me at least, but Eclipse often does it own building itself for reasons i don't know about)

So problem gone, without knowing why.
Thank you for your response.
Pete C.



On Friday, July 31, 2020, 02:20:03 PM PDT, Bob Jacobsen <rgj1927@...> wrote:




> On Jul 31, 2020, at 1:46 PM, Pete Cressman <pete_cressman@...> wrote:
>
> I have a problem I can't fix.
>
> import jmri.util.JUnitUtil; is not resolved in any test file where changes were made even after a real clean compile completes successfully. Curiously, the assertion fails on the last reference - JUnitUtil.tearDown();
>

Is there a more detailed error message?



> I am runing tests from eclipse and for quite some time now all tests have been prefaced with:
>
> INFO: Loading JUnit Platform configuration parameters from classpath resource [file:/C:/Git/JMRI/target/test-classes/junit-platform.properties].
> log4j:WARN No appenders could be found for logger (com.tngtech.archunit.core.PluginLoader).
> log4j:WARN Please initialize the log4j system properly.
> log4j:WARN See http://logging.apache.org/log4j/1.2/faq.html#noconfig for more info.
> Using org.netbeans.jemmy.drivers.DefaultDriverInstaller driver installer
> ERROR - Unable to store node identities: C:\Git\JMRI\temp\nodeIdentity.xml (The system cannot find the path specified) [AWT-EventQueue-0] jmri.util.node.NodeIdentity.saveIdentity()
> WARN  - Could not open/create prefs root node Software\JavaSoft\Prefs at root 0x80000002. Windows RegCreateKeyEx(...) returned error code 5. [AWT-EventQueue-0] java.util.prefs.log()
>
> Is there something I can do to fix this?

>

I think the "Could not open/create prefs root node Software\JavaSoft\Prefs” is a known Windows issue. Not sure if there’s a fix.

On the "Unable to store node identities’ message, could you check the the "C:\Git\JMRI\temp” directory exists?  It’s normally created by the tests when need, but perhaps something is going wrong with permissions, etc.

Bob


Bob Jacobsen
rgj1927@...






Re: jmri.util.JUnitUtil not found?

Bob Jacobsen
 

On Jul 31, 2020, at 1:46 PM, Pete Cressman <pete_cressman@...> wrote:

I have a problem I can't fix.

import jmri.util.JUnitUtil; is not resolved in any test file where changes were made even after a real clean compile completes successfully. Curiously, the assertion fails on the last reference - JUnitUtil.tearDown();
Is there a more detailed error message?


I am runing tests from eclipse and for quite some time now all tests have been prefaced with:

INFO: Loading JUnit Platform configuration parameters from classpath resource [file:/C:/Git/JMRI/target/test-classes/junit-platform.properties].
log4j:WARN No appenders could be found for logger (com.tngtech.archunit.core.PluginLoader).
log4j:WARN Please initialize the log4j system properly.
log4j:WARN See http://logging.apache.org/log4j/1.2/faq.html#noconfig for more info.
Using org.netbeans.jemmy.drivers.DefaultDriverInstaller driver installer
ERROR - Unable to store node identities: C:\Git\JMRI\temp\nodeIdentity.xml (The system cannot find the path specified) [AWT-EventQueue-0] jmri.util.node.NodeIdentity.saveIdentity()
WARN - Could not open/create prefs root node Software\JavaSoft\Prefs at root 0x80000002. Windows RegCreateKeyEx(...) returned error code 5. [AWT-EventQueue-0] java.util.prefs.log()

Is there something I can do to fix this?
I think the "Could not open/create prefs root node Software\JavaSoft\Prefs” is a known Windows issue. Not sure if there’s a fix.

On the "Unable to store node identities’ message, could you check the the "C:\Git\JMRI\temp” directory exists? It’s normally created by the tests when need, but perhaps something is going wrong with permissions, etc.

Bob


Bob Jacobsen
@BobJacobsen

jmri.util.JUnitUtil not found?

Pete Cressman
 

I have a problem I can't fix.

import jmri.util.JUnitUtil; is not resolved in any test file where changes were made even after a real clean compile completes successfully. Curiously, the assertion fails on the last reference - JUnitUtil.tearDown();

I am runing tests from eclipse and for quite some time now all tests have been prefaced with:

INFO: Loading JUnit Platform configuration parameters from classpath resource [file:/C:/Git/JMRI/target/test-classes/junit-platform.properties].
log4j:WARN No appenders could be found for logger (com.tngtech.archunit.core.PluginLoader).
log4j:WARN Please initialize the log4j system properly.
log4j:WARN See http://logging.apache.org/log4j/1.2/faq.html#noconfig for more info.
Using org.netbeans.jemmy.drivers.DefaultDriverInstaller driver installer
ERROR - Unable to store node identities: C:\Git\JMRI\temp\nodeIdentity.xml (The system cannot find the path specified) [AWT-EventQueue-0] jmri.util.node.NodeIdentity.saveIdentity()
WARN  - Could not open/create prefs root node Software\JavaSoft\Prefs at root 0x80000002. Windows RegCreateKeyEx(...) returned error code 5. [AWT-EventQueue-0] java.util.prefs.log()

Is there something I can do to fix this?
Thank you,
Pete C.

Re: Find some consensus

danielb987
 

You could add a foot note like this:

If a dispute cannot be resolved, the author of the PR may adress it on the JMRI developers mailing list. If needed, the owners of JMRI may merge the PR despite a review, but they will not do it unless there is a strong reason to do so.

Daniel


-------- Originalmeddelande --------
Från: Paul Bender <paul.bender@...>
Datum: 2020-07-26 05:07 (GMT+01:00)
Till: jmri@jmri-developers.groups.io
Rubrik: Re: [jmri-developers] Find some consensus


> On Jul 25, 2020, at 9:42 PM, danielb987 <db123@...> wrote:
>
> Ok. I see what you mean. I approve your changes and I withdraw my suggestion.

Your suggestion is a good one, I just don’t have any ideas on how disputes of this nature should be handled.

I am open to ideas.

Paul



Re: Find some consensus

Randall Wood <rhwood@...>
 

Regarding blocking a PR by requesting changes, and requiring that PRs not have changes requested before merging: it's possible for an owner to override that and merge anyways.

Regarding indicating a PR is ready to merge: it is an assumption built into the GitHub model that PRs are ready to merge unless otherwise indicated (i.e. by using WIP in the title, failing a CI check, does not contain sufficient approving reviews), so we do not need any other markers that a PR is ready to merge.

Regarding the use of labels: special privileges are required to set labels, so we can’t depend on their use by new or occasional contributors, so I would avoid using them in any workflow at this time.

On 25-Jul-2020, at 23:07, Paul Bender <paul.bender@...> wrote:


On Jul 25, 2020, at 9:42 PM, danielb987 <db123@...> wrote:

Ok. I see what you mean. I approve your changes and I withdraw my suggestion.
Your suggestion is a good one, I just don’t have any ideas on how disputes of this nature should be handled.

I am open to ideas.

Paul


Some cases to consider

Bob Jacobsen
 

A couple people have written and called me privately with questions. Based on the questions received, I’ve pulled some points together to share with the group. I haven’t been following jmri-developers for a while, pending whatever consensus arises. So the following might not be quite on-point for the discussion. Use these as you wish.



First, lets consider some cases where people might or might not have common views about how to deal with contributed code:

1) Somebody contributes a self-contained new feature which seems to work, doesn’t change or break anything existing, has lots of test coverage, passes the CI screens, and the code looks nice and clean. - This ones an easy no-brainer to start with, and I expect people think it clearly should be merged.

2) Same as (1), except it doesn’t have any tests at all. Should it be merged to get that feature in people’s hands? Should the author have to write tests before that happens? Just 1 (to pass CI), or lots (to improve various metrics)? Or should somebody else step up and add tests? Who? One of the advocates of complete testing? Somebody who agrees it’s a cool feature?

3) Same as (1), except that it doesn’t pass a bunch of the code-style CI tests, i.e. maybe unused variables or javadoc warnings or unchecked casts or …. Should the author have to cleanly fix all these & get CI passing? Or will somebody who really cares about those step up and fix them? Or would it be OK to just put @Suppress statements in the package-info.java files, suppress them all, and move on?

4) Same as (1), except that even though it has tests and passes CI, the code doesn’t look so good: Massive cut and paste, lots of casting, uses every number from 0 to 158 as a bare constant, sometimes in decimal and sometimes in hexadecimal, throws exceptions for normal returns across levels, Yet it’s a really cool feature much loved by users. Now what? Who does what?



Now a similar range on tooling/infrastructure changes:

A) A developer wants to include a new developer tool that requires _no_ change at all by others - it just works, has documentation, does not change any existing process, etc. I.e. a new Javadoc doclet that produces prettier output and can be distributed from JMRI GitHub. What needs to be considered before merging this?

B) As (A), but there’s no documentation about how to use it. Is that needed before merge? Does the author need to write it, or should somebody else step up? How good does it have to be?

C) As (A), but it requires installation and/or setup of some utilities to use it. E.g. another IDE that somebody wants to support or a tool for (optional) testing. Does that need documentation? Anything else?

D) As (A), except it’s a mandatory step. E.g. another preprocessor that has to be run to convert some new-format text for compilation, needed on every clean build. If this requires every developer to install something, is there some cost-benefit balance done? By who? What counts as cost? What counts as benefit?

E) As (A), but it’s a new “primary build tool”, which has the side effect of making existing build tools no longer equivalent. i.e. the change to IDE X means that command-line Maven builds are not longer building the same thing, or Ant builds are, or whatever. Is there some cost-benefit balance done here? By who? What counts as cost? What counts as benefit?

(I was discussing A-E with a developer yesterday, and he said “And what if I get burned by somebody breaking Eclipse build setup again, because I was away long enough to miss the discussion?”)



Sometimes there’s an overlap of the cases, when somebody wants to make a cross-cutting change that touches 50% or 80% or 100% of the files, either now or eventually. How is that evaluated?

i) A light syntactic migration, such as `new AList<Foo>` changed to `new AList<>`. Is it OK to make a migration like this in big chunks of files, even all at once? Just a few files? If somebody starts with just a few, do they have an obligation to complete it so that the code “has a consistent form”? Or does somebody else?

ii) A heavier syntactic migration, where tastes may differ. For example, changing anonymous inner classes to lambdas, or changing loops to functional stream operations. Should people just make these conversions as they write new code? Is it ok to migrate old code? Code that other people are known to be continuously working on, even if they don’t agree the new form is clearer? Can people still write the old forms if that makes them more productive? When is the new coolness a mandatory good, when is it a matter of taste and choice, and when is it a pretentious pain that we should kill with fire?

iii) We’ve had a convention for handing functional migration through deprecation cycles. But when do we have to wait the cycle and when can we just get on with making the changes? Should we mark the code someway to separate those? If so, who’s going to do that work for all that code? Is it the people advocating for this, or will some other people be encouraged / imposed on to do it as they work on the code? Do we need a new release system, and if so, will the proponents of that do the work of setting it set up, tested, documented, etc or should somebody else be expected to do that, including being criticized afterwards?

iv) If there’s a functional migration that’s universal, like a change to logging or an interface in the jmri package or the version of some run-time or test library, how does that get done? Should the proponents be given the very large burden of making the change all at once? That’s a lot of work. It’s even more work to do it slowly, one bit at a time, because of coordination and communication burdens. But that spreads it out in time, or across people. But _how_ do you equitably spread this work across people who don’t necessarily value the change over the things they actually want to work on? When can something be imposed, and when is it not worth it?



That’s a lot of items, probably too many to think through. Maybe you can tell what I think about them, maybe not, but I encourage you all to consider each one. They’re not hypothetical. Every single one of those has a concrete example in JMRI’s history. Some were routine, a few more were easy, but some of these were very very hard. People want to make changes, and other people don’t want to get stuck with the work and/or side-effects. And that happens in lots and lots of ways. It would be good to consider them now so as to have come context when the next time one comes up.


Underlying the differences of opinion seems to be a larger question:

What it the relative importance of each of the following in the future direction of JMRI:
- cool new CS features, structures and tools in use
- clean library structure that’s usable externally
- more tested and certified code as a part to the future
- cool new app features for model railroaders


100% of any of one those four is not OK. But is 25%/25%/25%/25% the goal? If at least half should be on features and future is important, are we talking 10%/10%/20%/50%? If some developers are interested in using JMRI to learn about new techniques and cool tools, does that mean 25%/25%/10%/40% is the goal?

And once you have an idea of which is important, are there any more things that can be added to the list? What fraction of the future direction should be assigned to each of your new ones?

Bob



Bob Jacobsen
@BobJacobsen

Re: Find some consensus

Paul Bender
 

On Jul 25, 2020, at 9:42 PM, danielb987 <db123@...> wrote:

Ok. I see what you mean. I approve your changes and I withdraw my suggestion.
Your suggestion is a good one, I just don’t have any ideas on how disputes of this nature should be handled.

I am open to ideas.

Paul

Re: Find some consensus

danielb987
 

Ok. I see what you mean. I approve your changes and I withdraw my suggestion.

Daniel

2020-07-26 02:41 skrev Paul Bender:

On 7/25/20 6:10 PM, danielb987 wrote:
I agree with the changes.
As I wrote in the PR, I suggest an additional point in the section
"Merging a PR":
If there are a review requesting changes, but the author and the
reviewer cannot resolve their dispute, then ????????
Note: I write question marks since I'm not sure how this should be
handled.
I am not either and I am especially not sure what to do when your
dispute is with one of the projects owners, or worse that a dispute is
between two of the project owners.  There are currently 5 of them,
though only 2 have really been active in any way in the last several years.
A dispute with one of the owners over a PR is at least in part what
started this thread in the first place, and I don't want to go down that
path again.
Paul

Re: Find some consensus

Paul Bender
 

On 7/25/20 6:10 PM, danielb987 wrote:
I agree with the changes.

As I wrote in the PR, I suggest an additional point in the section
"Merging a PR":
If there are a review requesting changes, but the author and the
reviewer cannot resolve their dispute, then ????????

Note: I write question marks since I'm not sure how this should be
handled.
I am not either and I am especially not sure what to do when your
dispute is with one of the projects owners, or worse that a dispute is
between two of the project owners.  There are currently 5 of them,
though only 2 have really been active in any way in the last several years.

A dispute with one of the owners over a PR is at least in part what
started this thread in the first place, and I don't want to go down that
path again.

Paul

Re: Find some consensus

danielb987
 

I agree with the changes.

As I wrote in the PR, I suggest an additional point in the section "Merging a PR":
If there are a review requesting changes, but the author and the reviewer cannot resolve their dispute, then ????????

Note: I write question marks since I'm not sure how this should be handled.

I agree with @rhwood that some PRs should never be merged, for different reasons, but I'm also concerned if a single reviewer can veto a PR that the rest of us wants to be merged.

Daniel


2020-07-25 20:53 skrev Paul Bender:

All,
Just to get this back on the right thread,
Daniel asked in another thread if I would get us a starting point for
the conversation on rules for PR reviews.
It turns out we already have a page documenting the PR merging process
we should be following. The existing page is here:
https://www.jmri.org/help/en/html/doc/Technical/gitadmin.shtml
I opened a PR to update that page with the items we have been
discussing about The PR approval process:
https://github.com/JMRI/JMRI/pull/8865
Please comment here about the changes suggested.
Paul
Links:
------
[1] https://jmri-developers.groups.io/g/jmri/message/4001
[2] https://groups.io/mt/75535614/1303822
[3] https://jmri-developers.groups.io/g/jmri/post
[4] https://jmri-developers.groups.io/g/jmri/editsub/1303822
[5] https://jmri-developers.groups.io/g/jmri/leave/defanged