Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [recommenders-dev] Argument completion: ping!?

Hi Cheng,

> We are now working off-line, fighting against hundreds of CheckStyle
> warnings...

Did you configure additional Checkstyle rules? AFAIK, the only
preconfigured rules check whether you are using Unix line endings and
whether each file ends with a line ending. At least the former
checkstyle warnings (which really are errors) should be fixable by
File/Convert Line Delimiters To/Unix.

But maybe you have configured additional checks. If so, I would be
curious to hear what you are checking for. Maybe we can then activate
these rules also for the other Code Recommenders projects. (As I said,
at the moment we don't check very much, but probably should check more.)

> As we failed the code review last time (sorry for our carelessness), we
> want to make the code ready before submit it.

IMHO, there is no such thing as a "failed" code review. The more people
reviewing the code the better. We all miss issues once in a while; no
harm in that.

So, why not upload your "git commit --amend"ed changes to Gerrit (just
make sure to include your "Change-Id:
I53f0e7f9533bce6b9af1e21ebbc0fb0d93cb0b95" somewhere in the commit
message, so that Gerrit knows that the change is not a new one but an
update to <https://git.eclipse.org/r/#/c/10681/>)?

If you want to make explicit that you yourselves don't consider the
current state of the change ready to be merged, just review it with
"-1". That way, everybody knows that you are not ready yet. But maybe
somebody is still interested enough in your code to review it even at
this stage (I certainly would) -- and then you get valuable feedback.

So, updates to a change set are no problem at all. Michael's Jayes 1.0.3
change <https://git.eclipse.org/r/#/c/10698/>, for example, is currently
in its 7th iteration -- and getting better all the time.

Hope this helps.

Andreas
-- 
Codetrails.com - the knowledge transfer company


Back to the top