[openstack-dev] [Solum] Dissecting the very first review

Adrian Otto adrian.otto at rackspace.com
Sat Nov 2 04:09:12 UTC 2013


On Nov 1, 2013, at 7:03 PM, Noorul Islam K M <noorul at noorul.com<mailto:noorul at noorul.com>>
 wrote:

Clark Boylan <clark.boylan at gmail.com<mailto:clark.boylan at gmail.com>> writes:

On Fri, Nov 1, 2013 at 6:29 PM, Noorul Islam K M <noorul at noorul.com<mailto:noorul at noorul.com>> wrote:


Now we have the first patch [1] merged into the repository using
OpenStack review process. I would like to bring into notice some minor
issues.

First of all I would like to thank [2] Swapnil for fixing the patch.

1. Look at patch set 3 and it changed the Author and also the Committer. I
  am not sure how that happened. I have been using gerrit outside of
  OpenStack and I never saw something like that.

2. Another strange part is that, the author date is Oct 1, 2013 12:53 PM

Also an ideal process for helping with others patch is discussed in [2].

Regards,
Noorul

[1] https://review.openstack.org/#/c/54877/
[2] http://www.mail-archive.com/openstack-dev@lists.openstack.org/msg05998.html

_______________________________________________
OpenStack-dev mailing list
OpenStack-dev at lists.openstack.org<mailto:OpenStack-dev at lists.openstack.org>
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

The Author, Committer, and Date are determined by the local git making
the commit. Gerrit is just displaying what was pushed to it. As a
reviewer if those items are important you can ask the author of a
patchset to push a new patchset that includes updated and potentially
more correct information. This may involve correcting local settings
(eg system clock) or you can override the values by passing the
'--author' and '--date' options to `git commit`.


The point I am trying to make is that, these things should be looked
into before the patch gets merged.

That date looks correct to me. I created/modified README.rst on October 1st, and that date was correctly imported into Stackforge from the original upstream repo. That's where it came from. I don't think anything is malfunctioning. The "committer" is labeled as:

Swapnil Kulkarni<https://review.openstack.org/#/dashboard/7051><swapnilkulkarni2608 at gmail.com<mailto:swapnilkulkarni2608 at gmail.com>>Nov 1, 2013 12:27 PM

That also looks correct, because that's the author who tweaked the commit so it would build, which is what I approved for merge. I recognize that the root cause of the Verify trouble were missing newline characters in the initial import, and had nothing to do with the new files being added with patch set 1. Let's move past this one together and continue supporting each other so we can continue this great forward momentum.

Thanks,

Adrian


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20131102/0a73df41/attachment.html>


More information about the OpenStack-dev mailing list