Open Stack

----- Original Message -----
> From: "Martin Geisler" <martin at geisler.net>
> To: openstack-dev at lists.openstack.org
> Sent: Wednesday, May 28, 2014 11:35:02 AM
> Subject: Re: [openstack-dev] Specifying file encoding
> 
> Ryan Brady <rbrady at redhat.com> writes:
> 
> > ----- Original Message -----
> >> From: "Pádraig Brady" <P at draigBrady.com>
> >> To: "OpenStack Development Mailing List (not for usage questions)"
> >> <openstack-dev at 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 at 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
> > <licensing at 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 that simply
update the line for Emacs compatibility.

> 
> 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.  

Cheers,

Ryan

> --
> Martin Geisler
> 
> https://plus.google.com/+MartinGeisler/
> 
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>