You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Tomoe Sugihara <to...@midokura.com> on 2012/07/31 10:29:48 UTC
Review Request: Fix the agent path according to
7a0a9231c355fee42c67799abe111edcd79998bb
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6227/
-----------------------------------------------------------
Review request for cloudstack and edison su.
Description
-------
Fix the agent path according to 7a0a9231c355fee42c67799abe111edcd79998bb
Signed-off-by: Tomoe Sugihara <to...@midokura.com>
Diffs
-----
python/lib/cloudutils/serviceConfig.py 539e26ae7322934de08576918a376c0487157d97
Diff: https://reviews.apache.org/r/6227/diff/
Testing
-------
Thanks,
Tomoe Sugihara
Re: Review Request: Fix the agent path according to
7a0a9231c355fee42c67799abe111edcd79998bb
Posted by Tomoe Sugihara <to...@midokura.com>.
> On July 31, 2012, 9:27 a.m., Prasanna Santhanam wrote:
> > Bug Report: http://bugs.cloudstack.org/browse/CS-15776
> >
> > Saw quite a few whitespace changes as well. Ran a pychecker on the patch and it looks fine.
> >
> >
>
> Tomoe Sugihara wrote:
> Actually the bug report was similar but for a different review. I didn't create one for this review.
>
> As for the trailing spaces, I wasn't sure if I should clean that up, but I thought it's a good practice in general to get rid of them.
> Let me know if I should leave them.
>
> Prasanna Santhanam wrote:
> Absolutely - it is a good practice. Even better if we can move all python code to PEP8 style. Just mentioned the pychecker test since python is picky about spaces. Wanted to make sure that we didn't have any space errors. I've pushed the fix. Thanks.
>
> David Nalley wrote:
> No please do clean up as you go.
OK, will do.
In that case, I'd recommend to configure on-save hook to cleanup trailing spaces on your editor or IDE.
- Tomoe
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6227/#review9632
-----------------------------------------------------------
On July 31, 2012, 8:29 a.m., Tomoe Sugihara wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6227/
> -----------------------------------------------------------
>
> (Updated July 31, 2012, 8:29 a.m.)
>
>
> Review request for cloudstack and edison su.
>
>
> Description
> -------
>
> Fix the agent path according to 7a0a9231c355fee42c67799abe111edcd79998bb
>
> Signed-off-by: Tomoe Sugihara <to...@midokura.com>
>
>
> Diffs
> -----
>
> python/lib/cloudutils/serviceConfig.py 539e26ae7322934de08576918a376c0487157d97
>
> Diff: https://reviews.apache.org/r/6227/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Tomoe Sugihara
>
>
Re: Review Request: Fix the agent path according to
7a0a9231c355fee42c67799abe111edcd79998bb
Posted by David Nalley <da...@gnsa.us>.
> On July 31, 2012, 9:27 a.m., Prasanna Santhanam wrote:
> > Bug Report: http://bugs.cloudstack.org/browse/CS-15776
> >
> > Saw quite a few whitespace changes as well. Ran a pychecker on the patch and it looks fine.
> >
> >
>
> Tomoe Sugihara wrote:
> Actually the bug report was similar but for a different review. I didn't create one for this review.
>
> As for the trailing spaces, I wasn't sure if I should clean that up, but I thought it's a good practice in general to get rid of them.
> Let me know if I should leave them.
>
> Prasanna Santhanam wrote:
> Absolutely - it is a good practice. Even better if we can move all python code to PEP8 style. Just mentioned the pychecker test since python is picky about spaces. Wanted to make sure that we didn't have any space errors. I've pushed the fix. Thanks.
No please do clean up as you go.
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6227/#review9632
-----------------------------------------------------------
On July 31, 2012, 8:29 a.m., Tomoe Sugihara wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6227/
> -----------------------------------------------------------
>
> (Updated July 31, 2012, 8:29 a.m.)
>
>
> Review request for cloudstack and edison su.
>
>
> Description
> -------
>
> Fix the agent path according to 7a0a9231c355fee42c67799abe111edcd79998bb
>
> Signed-off-by: Tomoe Sugihara <to...@midokura.com>
>
>
> Diffs
> -----
>
> python/lib/cloudutils/serviceConfig.py 539e26ae7322934de08576918a376c0487157d97
>
> Diff: https://reviews.apache.org/r/6227/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Tomoe Sugihara
>
>
Re: Review Request: Fix the agent path according to
7a0a9231c355fee42c67799abe111edcd79998bb
Posted by Tomoe Sugihara <to...@midokura.com>.
> On July 31, 2012, 9:27 a.m., Prasanna Santhanam wrote:
> > Bug Report: http://bugs.cloudstack.org/browse/CS-15776
> >
> > Saw quite a few whitespace changes as well. Ran a pychecker on the patch and it looks fine.
> >
> >
Actually the bug report was similar but for a different review. I didn't create one for this review.
As for the trailing spaces, I wasn't sure if I should clean that up, but I thought it's a good practice in general to get rid of them.
Let me know if I should leave them.
- Tomoe
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6227/#review9632
-----------------------------------------------------------
On July 31, 2012, 8:29 a.m., Tomoe Sugihara wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6227/
> -----------------------------------------------------------
>
> (Updated July 31, 2012, 8:29 a.m.)
>
>
> Review request for cloudstack and edison su.
>
>
> Description
> -------
>
> Fix the agent path according to 7a0a9231c355fee42c67799abe111edcd79998bb
>
> Signed-off-by: Tomoe Sugihara <to...@midokura.com>
>
>
> Diffs
> -----
>
> python/lib/cloudutils/serviceConfig.py 539e26ae7322934de08576918a376c0487157d97
>
> Diff: https://reviews.apache.org/r/6227/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Tomoe Sugihara
>
>
Re: Review Request: Fix the agent path according to
7a0a9231c355fee42c67799abe111edcd79998bb
Posted by Prasanna Santhanam <Pr...@citrix.com>.
> On July 31, 2012, 9:27 a.m., Prasanna Santhanam wrote:
> > Bug Report: http://bugs.cloudstack.org/browse/CS-15776
> >
> > Saw quite a few whitespace changes as well. Ran a pychecker on the patch and it looks fine.
> >
> >
>
> Tomoe Sugihara wrote:
> Actually the bug report was similar but for a different review. I didn't create one for this review.
>
> As for the trailing spaces, I wasn't sure if I should clean that up, but I thought it's a good practice in general to get rid of them.
> Let me know if I should leave them.
Absolutely - it is a good practice. Even better if we can move all python code to PEP8 style. Just mentioned the pychecker test since python is picky about spaces. Wanted to make sure that we didn't have any space errors. I've pushed the fix. Thanks.
- Prasanna
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6227/#review9632
-----------------------------------------------------------
On July 31, 2012, 8:29 a.m., Tomoe Sugihara wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6227/
> -----------------------------------------------------------
>
> (Updated July 31, 2012, 8:29 a.m.)
>
>
> Review request for cloudstack and edison su.
>
>
> Description
> -------
>
> Fix the agent path according to 7a0a9231c355fee42c67799abe111edcd79998bb
>
> Signed-off-by: Tomoe Sugihara <to...@midokura.com>
>
>
> Diffs
> -----
>
> python/lib/cloudutils/serviceConfig.py 539e26ae7322934de08576918a376c0487157d97
>
> Diff: https://reviews.apache.org/r/6227/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Tomoe Sugihara
>
>
Re: Review Request: Fix the agent path according to
7a0a9231c355fee42c67799abe111edcd79998bb
Posted by Prasanna Santhanam <Pr...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6227/#review9632
-----------------------------------------------------------
Ship it!
Bug Report: http://bugs.cloudstack.org/browse/CS-15776
Saw quite a few whitespace changes as well. Ran a pychecker on the patch and it looks fine.
- Prasanna Santhanam
On July 31, 2012, 8:29 a.m., Tomoe Sugihara wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6227/
> -----------------------------------------------------------
>
> (Updated July 31, 2012, 8:29 a.m.)
>
>
> Review request for cloudstack and edison su.
>
>
> Description
> -------
>
> Fix the agent path according to 7a0a9231c355fee42c67799abe111edcd79998bb
>
> Signed-off-by: Tomoe Sugihara <to...@midokura.com>
>
>
> Diffs
> -----
>
> python/lib/cloudutils/serviceConfig.py 539e26ae7322934de08576918a376c0487157d97
>
> Diff: https://reviews.apache.org/r/6227/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Tomoe Sugihara
>
>