Re: [openstack-dev] Specifying file encoding

2014-05-29 Thread Martin Geisler
Ryan Brady rbr...@redhat.com writes:

 Would you want to merge the patches that simply remove the unneeded
 lines and then let me followup with patches that remove © along with
 the then unnecessary coding lines?

 If I was in your position, I'd update the patches that remove lines to
 include all of the affected files and also remove the ©. I'd abandon
 the patches that simply update the line for Emacs compatibility.

Done: https://review.openstack.org/96123/ Let me know what you think.

 I'm asking since it seems that Gerrit encourages a different style of
 development than most other projects I know -- single large commits
 instead of a series of smaller commits, each one logical step
 building on the previous.
 

 When patches are too complex, breaking them down makes it easier to
 review, easier to test and easier to revert. In this case, I don't
 think you're adding complexity by changing a line and a character in
 comments for each file in the scope of a project. Opinions may vary
 project to project.

Yeah, that makes sense and I've already heard some differnet opinions.
That's of course fine with me!

I'm a developer in the Mercurial project, and we have strict policies
about patches doing one thing only. Exaclty what one thing can then
sometimes be up for debate :)

Thanks for the help so far!

-- 
Martin Geisler

https://plus.google.com/+MartinGeisler/


pgpzwz81KBikm.pgp
Description: PGP signature
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] Specifying file encoding

2014-05-28 Thread Pádraig Brady
On 05/28/2014 08:16 AM, Martin Geisler wrote:
 Hi everybody,
 
 I'm trying to get my feet wet with OpenStack development, so I recently
 tried to submit some small patches. One small thing I noticed was that
 some files used
 
   # -*- encoding: utf-8 -*-
 
 to specify the file encoding for both Python and Emacs. Unfortunately,
 Emacs expects you to set coding, not encoding. Python is fine with
 either. I submitted a set of patches for this:
 
 * https://review.openstack.org/95862
 * https://review.openstack.org/95864
 * https://review.openstack.org/95865
 * https://review.openstack.org/95869
 * https://review.openstack.org/95871
 * https://review.openstack.org/95880
 * https://review.openstack.org/95882
 * https://review.openstack.org/95886
 
 It was pointed out to me that such a change ought to be coordinated
 better via bug(s) or the mailinglist, so here I am :)

This is valid change.
I don't see why there is any question
as it only improves the situation for emacs
which will pop up an error when trying to edit these files.

You could create a bug I suppose
to reference all the changes though I don't
think that's mandatory since this isn't user facing.

cheers,
Pádraig.

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] Specifying file encoding

2014-05-28 Thread Martin Geisler
Pádraig Brady p...@draigbrady.com writes:

 On 05/28/2014 08:16 AM, Martin Geisler wrote:
 Hi everybody,
 
 I'm trying to get my feet wet with OpenStack development, so I recently
 tried to submit some small patches. One small thing I noticed was that
 some files used
 
   # -*- encoding: utf-8 -*-
 
 to specify the file encoding for both Python and Emacs. Unfortunately,
 Emacs expects you to set coding, not encoding. Python is fine with
 either. I submitted a set of patches for this:
 
 * https://review.openstack.org/95862
 * https://review.openstack.org/95864
 * https://review.openstack.org/95865
 * https://review.openstack.org/95869
 * https://review.openstack.org/95871
 * https://review.openstack.org/95880
 * https://review.openstack.org/95882
 * https://review.openstack.org/95886
 
 It was pointed out to me that such a change ought to be coordinated
 better via bug(s) or the mailinglist, so here I am :)

 This is valid change.
 I don't see why there is any question
 as it only improves the situation for emacs
 which will pop up an error when trying to edit these files.

Yes, exactly :)

It's also worth noting that the files *already* use Emacs-specific
markup in the form of the -*- markers. Python doesn't care about those
at all, but Emacs relies on them.

The reviewers in https://review.openstack.org/95886/ suggests removing
the coding lines completely and also remove non-ASCII characters as
necessary to make that possible.

I can definitely do that instead if people like that better. The only
problem I see is that we'll have to hope that © is the only non-ASCII
character -- I haven't checked if that's the case yet, but it's not
uncommon to find non-ASCII characters in author names as well.

 You could create a bug I suppose to reference all the changes though I
 don't think that's mandatory since this isn't user facing.

Okay, I'm not really familiar with the process around here. I hoped I
could just make some commits and submit them for review :)

-- 
Martin Geisler

https://plus.google.com/+MartinGeisler/


pgpuijH_jhThC.pgp
Description: PGP signature
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] Specifying file encoding

2014-05-28 Thread Martin Geisler
Ryan Brady rbr...@redhat.com writes:

 - Original Message -
 From: Pádraig Brady p...@draigbrady.com
 To: OpenStack Development Mailing List (not for usage questions)
 openstack-dev@lists.openstack.org
 Sent: Wednesday, May 28, 2014 6:28:28 AM
 Subject: Re: [openstack-dev] Specifying file encoding
 
 On 05/28/2014 08:16 AM, Martin Geisler wrote:
  Hi everybody,
  
  I'm trying to get my feet wet with OpenStack development, 

 Welcome aboard!  Thanks for contributing.  :)

 so I recently
  tried to submit some small patches. One small thing I noticed was that
  some files used
  
# -*- encoding: utf-8 -*-
  
  to specify the file encoding for both Python and Emacs. Unfortunately,
  Emacs expects you to set coding, not encoding. Python is fine with
  either. I submitted a set of patches for this:
  
  * https://review.openstack.org/95862
  * https://review.openstack.org/95864
  * https://review.openstack.org/95865
  * https://review.openstack.org/95869
  * https://review.openstack.org/95871
  * https://review.openstack.org/95880
  * https://review.openstack.org/95882
  * https://review.openstack.org/95886
  
  It was pointed out to me that such a change ought to be coordinated
  better via bug(s) or the mailinglist, so here I am :)
 
 This is valid change.
 I don't see why there is any question
 as it only improves the situation for emacs
 which will pop up an error when trying to edit these files.

 I guess I approach this differently. When I saw this patch, my first
 thought was to validate if the line being changed needed to exist at
 all.

That makes a lot of sense!

 If the file has valid non-ascii characters that effect its execution,
 are absolutely required for documentation to convey a specific
 meaning, or in strings that need to translate, then I agree the change
 is valid. But in the case the characters in the file can be changed,
 it seems like the bug is the extra encoding comment itself.

I see what you mean -- I also try to keep my files just ASCII for
convenience (even though I'm from Denmark where we have the extra three
vowels æ, ø, and å). As a brand new contributor, changing copyright
statements seemed like a bigger change than updating the coding line :)

 Taking tuskar for example, the files in question seem to only need
 this encoding line to support the copyright symbol.

 [rb@localhost tuskar]$ grep -R -i -P [^\x00-\x7F] ./*
 Binary file ./doc/source/api/img/model_v2.jpg matches
 Binary file ./doc/source/api/img/model_v4.odg matches
 Binary file ./doc/source/api/img/model.odg matches
 Binary file ./doc/source/api/img/model_v3.jpg matches
 Binary file ./doc/source/api/img/model_v3.odg matches
 Binary file ./doc/source/api/img/model_v2.odg matches
 Binary file ./doc/source/api/img/model_v4.jpg matches
 Binary file ./doc/source/api/img/model_v1.jpg matches
 Binary file ./doc/source/_static/header_bg.jpg matches
 Binary file ./doc/source/_static/header-line.gif matches
 Binary file ./doc/source/_static/openstack_logo.png matches
 ./tuskar/api/hooks.py:# Copyright © 2012 New Dream Network, LLC (DreamHost)
 ./tuskar/api/app.py:# Copyright © 2012 New Dream Network, LLC (DreamHost)
 ./tuskar/api/acl.py:# Copyright © 2012 New Dream Network, LLC (DreamHost)
 ./tuskar/common/service.py:# Copyright © 2012 eNovance 
 licens...@enovance.com
 ./tuskar/tests/api/api.py:# Copyright © 2012 New Dream Network, LLC 
 (DreamHost)


 In the U.S., copyright notices haven't really been needed since 1989.
 You also only need to include
 one instance of the symbol, Copyright or copr [1]. If the
 requirements for copyright are different
 outside the U.S., then I hope we capture that in the copyright wiki
 page [2]. Maybe the current info
 in that wiki needs to be updated or at least notated as to why the
 specific notice text is suggested.

 Unless there is another valid requirement to keep the © in the files,
 I think it's best if we just remove them altogether and eliminate the
 need to add the encoding comments at all.

Sounds good to me. I'll update my script to do this and rework the patch
sets. I've made most patches as two: one that removes coding lines that
are currently redudant and one that adjusts the remaining lines to make
Emacs happy.

Would you want to merge the patches that simply remove the unneeded
lines and then let me followup with patches that remove © along with the
then unnecessary coding lines?

I'm asking since it seems that Gerrit encourages a different style of
development than most other projects I know -- single large commits
instead of a series of smaller commits, each one logical step building
on the previous.

-- 
Martin Geisler

https://plus.google.com/+MartinGeisler/


pgpael_3tsbag.pgp
Description: PGP signature
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] Specifying file encoding

2014-05-28 Thread Ryan Brady


- Original Message -
 From: Martin Geisler mar...@geisler.net
 To: openstack-dev@lists.openstack.org
 Sent: Wednesday, May 28, 2014 11:35:02 AM
 Subject: Re: [openstack-dev] Specifying file encoding
 
 Ryan Brady rbr...@redhat.com writes:
 
  - Original Message -
  From: Pádraig Brady p...@draigbrady.com
  To: OpenStack Development Mailing List (not for usage questions)
  openstack-dev@lists.openstack.org
  Sent: Wednesday, May 28, 2014 6:28:28 AM
  Subject: Re: [openstack-dev] Specifying file encoding
  
  On 05/28/2014 08:16 AM, Martin Geisler wrote:
   Hi everybody,
   
   I'm trying to get my feet wet with OpenStack development,
 
  Welcome aboard!  Thanks for contributing.  :)
 
  so I recently
   tried to submit some small patches. One small thing I noticed was that
   some files used
   
 # -*- encoding: utf-8 -*-
   
   to specify the file encoding for both Python and Emacs. Unfortunately,
   Emacs expects you to set coding, not encoding. Python is fine with
   either. I submitted a set of patches for this:
   
   * https://review.openstack.org/95862
   * https://review.openstack.org/95864
   * https://review.openstack.org/95865
   * https://review.openstack.org/95869
   * https://review.openstack.org/95871
   * https://review.openstack.org/95880
   * https://review.openstack.org/95882
   * https://review.openstack.org/95886
   
   It was pointed out to me that such a change ought to be coordinated
   better via bug(s) or the mailinglist, so here I am :)
  
  This is valid change.
  I don't see why there is any question
  as it only improves the situation for emacs
  which will pop up an error when trying to edit these files.
 
  I guess I approach this differently. When I saw this patch, my first
  thought was to validate if the line being changed needed to exist at
  all.
 
 That makes a lot of sense!
 
  If the file has valid non-ascii characters that effect its execution,
  are absolutely required for documentation to convey a specific
  meaning, or in strings that need to translate, then I agree the change
  is valid. But in the case the characters in the file can be changed,
  it seems like the bug is the extra encoding comment itself.
 
 I see what you mean -- I also try to keep my files just ASCII for
 convenience (even though I'm from Denmark where we have the extra three
 vowels æ, ø, and å). As a brand new contributor, changing copyright
 statements seemed like a bigger change than updating the coding line :)
 

It helps me to think about effort, value, complexity, future maintenance,
and dependencies when evaluating changes.

  Taking tuskar for example, the files in question seem to only need
  this encoding line to support the copyright symbol.
 
  [rb@localhost tuskar]$ grep -R -i -P [^\x00-\x7F] ./*
  Binary file ./doc/source/api/img/model_v2.jpg matches
  Binary file ./doc/source/api/img/model_v4.odg matches
  Binary file ./doc/source/api/img/model.odg matches
  Binary file ./doc/source/api/img/model_v3.jpg matches
  Binary file ./doc/source/api/img/model_v3.odg matches
  Binary file ./doc/source/api/img/model_v2.odg matches
  Binary file ./doc/source/api/img/model_v4.jpg matches
  Binary file ./doc/source/api/img/model_v1.jpg matches
  Binary file ./doc/source/_static/header_bg.jpg matches
  Binary file ./doc/source/_static/header-line.gif matches
  Binary file ./doc/source/_static/openstack_logo.png matches
  ./tuskar/api/hooks.py:# Copyright © 2012 New Dream Network, LLC (DreamHost)
  ./tuskar/api/app.py:# Copyright © 2012 New Dream Network, LLC (DreamHost)
  ./tuskar/api/acl.py:# Copyright © 2012 New Dream Network, LLC (DreamHost)
  ./tuskar/common/service.py:# Copyright © 2012 eNovance
  licens...@enovance.com
  ./tuskar/tests/api/api.py:# Copyright © 2012 New Dream Network, LLC
  (DreamHost)
 
 
  In the U.S., copyright notices haven't really been needed since 1989.
  You also only need to include
  one instance of the symbol, Copyright or copr [1]. If the
  requirements for copyright are different
  outside the U.S., then I hope we capture that in the copyright wiki
  page [2]. Maybe the current info
  in that wiki needs to be updated or at least notated as to why the
  specific notice text is suggested.
 
  Unless there is another valid requirement to keep the © in the files,
  I think it's best if we just remove them altogether and eliminate the
  need to add the encoding comments at all.
 
 Sounds good to me. I'll update my script to do this and rework the patch
 sets. I've made most patches as two: one that removes coding lines that
 are currently redudant and one that adjusts the remaining lines to make
 Emacs happy.
 
 Would you want to merge the patches that simply remove the unneeded
 lines and then let me followup with patches that remove © along with the
 then unnecessary coding lines?

If I was in your position, I'd update the patches that remove lines to include 
all
of the affected files and also remove the ©.  I'd abandon the patches