Well, the problem is that different people disagree on what's "meaningful"
in this context. For example:
See PersistentPartitionHangsDuringRestartRegressionTest.java
* /***
* * RegressionTest for bug 42226. <br>*
* * 1. Member A has the bucket <br>*
* * 2. Member B starts creating the bucket. It tells member A that it
hosts the bucket <br>*
* * 3. Member A crashes <br>*
* * 4. Member B destroys the bucket and throws a partition offline
exception, because it wasn't*
* * able to complete initialization. <br>*
* * 5. Member A recovers, and gets stuck waiting for member B.*
* **
* * <p>*
* * TRAC 42226: recycled VM hangs during re-start while waiting for
Partition to come online (after*
* * Controller VM sees unexpected PartitionOffLineException while doing
ops)*
* */*
* @Test*
* public void doesNotWaitForPreviousInstanceOfOnlineServer() throws
Exception {*
I personally would NOT remove the references to "42226" from the above, and
here's why:
1) The javadoc clearly states the what and how of this bug
2) The 2nd paragraph includes the full summary from the old TRAC ticket
3) Quite a few people working on Geode do have access to TRAC which means
anyone in the community can request us to go dig up further history about
this bug to share
If we systematically delete all TRAC #s from javadocs and comments then
community developers will lose that opportunity. While it's true that most
of the time most developers will probably never need or want that history,
I would not say it's never going to be valuable.
When I'm working on code that references old bugs (especially regression
tests like the one above), I frequently pull up TRAC and read about the
bug. I also tend to pull up the old pre-Apache git repo and read through
old commit messages. Maybe I'm the only one who does this.
Despite my point of view, I don't feel very strongly about it. If you guys
really think that the TRAC #s distract or confuse you too much and you
can't foresee the need to ask someone to pull up history on a ticket, then
you should submit a PR to "remove all TRAC #s" from the code base. I won't
disapprove it -- I can always look at older revisions of files like
PersistentPartitionHangsDuringRestartRegressionTest to find the old TRAC #.
I previously submitted PRs to rename all test classes and test methods from
names like test42226 to meaningful names so I support the overall intent.
On Tue, Feb 19, 2019 at 5:21 PM Jacob Barrett <jbarr...@pivotal.io> wrote:
> Comments that don’t provide meaningful context beyond what is already
> expressed in the code should be removed. A number to a system that the
> general public can’t access is not meaningful. Delete or replace with
> meaningful comment.
>
> -jake
>
>
> > On Feb 19, 2019, at 1:41 PM, Michael Oleske <mole...@pivotal.io> wrote:
> >
> > Hey Geode Dev Friends!
> >
> > I was reviewing a PR (this one https://github.com/apache/geode/pull/3197
> )
> > and made a note that maybe we should remove comments that make references
> > to bug and trac numbers that people cannot reach (like me for one). Kirk
> > mentioned that some people (like him) have access to those bugs and have
> > proven helpful for some history. So there is this balance between noise
> > (people who cannot access those old issues) and getting context (people
> who
> > can access those issues).
> >
> > So I guess my point is to start a discussion on what a path forward might
> > be (if any)? My opinion is that they are noise and we should remove
> them.
> > If someone has access to the original issue, then making sure there is a
> > test case covering it should be done. Then it makes even more sense to
> me
> > to remove the comment.
> >
> > -michael
>
