Re: [Kicad-developers] A reminder on Git commit comments

2018-05-12 Thread Carsten Schoenert
Hello Rene,

I'm answering with a it of delay, to much to do in to less time.
We are going to be oftopic for this list and should do discussions about
these specific topics on kicad-lib-committers ... but unfortunately this
list is mostly dead and unused.

Am 03.05.2018 um 12:57 schrieb Rene Pöschl:
...
>> that's unfortunately true for most of contributors or contributions no
>> matter what kind of software or project and that's a social challenge we
>> all need to deal with.
> 
> That's the difference you miss. Software engineers are at least 
> interested in learning about version control systems. After all they can 
> use that skill in their career. Electrical engineers typically do not 
> use it.

I'm fully aware about this as I see such behavior mostly every day on my
day job. The unwillingness of some "engineers" to not use and accept a
SCM on present days is becoming a bigger problem from day to day. But
it's wrong to accept this circumstance in my eyes. If you try to ignore
this your work won't ever became easier or more productive. I see mostly
colleges that are happy to do better job if you explain and help them to
understand why they should change their workflow.

...
> Most of the open pull requests have been reviewed and a fix from the 
> contributor size is necessary.

Really?
Isn't it sometimes, I'd say mostly, not easier if you as the final
decision taking person would fix smaller things directly?
My experience in similar situations is that contributors are happy to
give a initial point or contribution but not really care how their idea
or work is added later and also doesn't care much about if their name is
listed in the contribution (for the reason you wrote above). But of
course we list the origin person behind a change.
I mean, you all as maintainers of the libraries will probably do a much
quicker job on doing the requested and needed changes on pull requests.
OTOH this means you need to break out the static GitHub WebUI.

But yes, some certain contributions are to poor or get them into a
reasonable state by yours.

...
> I know the technicalities of git. But it doesn't help much if the thing 
> you are working on is not really well suited to be handled by git.

That's what I already stated.

> A good example is that from time to time the save algorithm (of
> kicad) seems to randomly change. (Insert whites-paces where they
> never where before, change the ordering of lines, ...)

For sure true. To avoid such unneeded changes some autotests need to be
added to the KiCad builds. But $SOMEONE needs to implement this then.
Should not be that difficult to add something like this after the
successful build of KiCad in the Jenkins machinery.

> Again we are not dealing with human created text files. Merging is not 
> really possible so we loose a large part of what can be done with git. 
> (So if you have someone who knows git and git only he can do exactly 
> nothing. The person needs to be able to read kicad files.)

I think it's now clear that the current workflow to add symbols and
footprints isn't optimal and efficient. People who want's to contribute
need to have a good knowledge about KiCads internals, need to deal right
with the QS work, with git VCS commands *and* then with the GitHub PR
workflow. That's far away from easy and quick.

...
> I have for my self compiled a list of features required for any tool 
> handling the official lib:
> - We need some way of sharing the data (oviosly)
> - We need some sort of staging area for users to commit changes for 
> review (we use pull requests for this)
> - We need a way to run automatic tests on the contribution (continued 
> integration support within pull requests)
> - We need a way to share screenshots and or technical drawings (comment 
> section of the pull request)
> - We need a way to give feedback (both in text and image form -> again 
> comment section of a pull request)
> - We need a way to track what exactly the contributor changed after 
> feedback has been given (well this is where git comes in handy.)
> - And after everything is ok we need a way to get the changes into the 
> main "repo" without risking overwriting other peoples contributions. 
> (The merge step)
> 
> So yes git and github have their limitations but i can't think of a 
> better tool right now.

Agreed. But knowing what the current problems are help to look and find
a better solution. Here are some points that going around in my head for
some times now.

I was thinking about a website with some kind of back end that is using
HTML5 technology to visualize the various contributions made by
uploaders and also to get such contribution easier and quicker into
place. How could this work?

A contributor is uploading (directly from the symboleditor?) to the
WebUI. Some JS or Python voodoo is taking the upload and producing a
Canvas element that is shown on the website, no need to upload a PNG or
JPG file. A additional link to the documentation or direct uploaded PDF

Re: [Kicad-developers] A reminder on Git commit comments

2018-05-03 Thread Rene Pöschl

On 03/05/18 08:31, Carsten Schoenert wrote:

Hello Rene,

Am 02.05.2018 um 14:57 schrieb Rene Pöschl:

Most of our contributors have no previous experience with git. So we
really can not expect them to understand anything beyond the github
workflow. (They are experts in electronics. Not IT) I also don't believe
we should burden the contributors with too much with this.

that's unfortunately true for most of contributors or contributions no
matter what kind of software or project and that's a social challenge we
all need to deal with.


That's the difference you miss. Software engineers are at least 
interested in learning about version control systems. After all they can 
use that skill in their career. Electrical engineers typically do not 
use it. They also normally do not get into contact with it in their 
education. And source code is a lot easier to merge. Merging two 
different contributions to the same symbol lib is a nightmare.

This is even true for the maintainers. They are selected because they
understand how to make good symbols and footprints. (Understanding of
industry standards, limitations of manufacturing processes and the
limitations of kicad)

Reading this thread and following from time to time some library issue
on GitHub (only by reading) I see this point in another light. It's up
to the maintainers to get fulfilled the requirements of the project if
he is accepting a pull request.

I see long lists of pull request on GitHub for the libraries which, and
this list seems even to increase more over the last months which is a
good sign I think, which waiting a review from one of the people which
are allowed to do merge requests. OTOH I see also commit messages as
Reece has pointed out. So for me it looks like there are two problems
which interacting together.


Most of the open pull requests have been reviewed and a fix from the 
contributor size is necessary.



1. The maintainers (you mean above I assume), which are needed to make
QS on new or modified parts based on the library standards, are also
"only" volunteering by there free time and this brings long waiting
queues even for simply contributions. This shows a lack of available
human resources with a deep understanding what the given standards are
mean on technical work.


Again: typically your contribution is reviewed within a few days. But i 
can't remember even one contribution (by a casual contributor) that i 
could merge after the first review. (Even with the fact that we have 
test scripts that take care of a lot of things before we librarians come 
into play.) There is just too much to consider.

2. There are also some library maintaining people needed which know how
to do the technical git work right. And this means mostly you sometimes
have to leave the world of platforms like GitHub, GitLab etc if needed.
If you know what a git merge really is and how a GitHub PR is
technically working under the hood you are able to solve quickly
contributions which otherwise need to wait until someone with a better
technical understanding will provide something useful in the issue
tracker on GitHub. Over the time this can become a serious problem for a
project as people can get quickly frustrated and wont contributing in
future times.
I know the technicalities of git. But it doesn't help much if the thing 
you are working on is not really well suited to be handled by git. A 
good example is that from time to time the save algorithm (of kicad) 
seems to randomly change. (Insert whites-paces where they never where 
before, change the ordering of lines, ...)


Again we are not dealing with human created text files. Merging is not 
really possible so we loose a large part of what can be done with git. 
(So if you have someone who knows git and git only he can do exactly 
nothing. The person needs to be able to read kicad files.)



For a lot of our casual contributors the current way of contributing is
already too complex. I have seen a lot of them simply give up as soon as
git did something they did not expect. This is especially common on the
symbol lib side as it is likely that the contributor needs to do a
manual merge because somebody else touched the same lib.

Also this problem isn't new and is related to git can't handle big
rather complex text files well if two persons have changed such a file.
git was developed for source code not for handling large text based
files. In the end you will come to the conclusion that git isn't the
best tool here but it's also the best we have.
For me than the workflow heavily based direct on plain git isn't the
correct one.


I have for my self compiled a list of features required for any tool 
handling the official lib:

- We need some way of sharing the data (oviosly)
- We need some sort of staging area for users to commit changes for 
review (we use pull requests for this)
- We need a way to run automatic tests on the contribution (continued 
integration support within pull requests)
- We need a 

Re: [Kicad-developers] A reminder on Git commit comments

2018-05-03 Thread Martijn Kuipers
FWIW, there are a few software packages out there that can help with git and 
are free to use with open source projects, such as, Gitkraken and Smartgit.

I am not affiliated with any of them, but I have used them both. They are 
pretty much similar for the most common use cases and both provide a 
(hopefully) clearer view into the tree

/Martijn
P.S. This is not to start a flamewar. I apologise for the noise to the 
git-masters around.

> On 3 May 2018, at 07:31, Carsten Schoenert  wrote:
> 
> Hello Rene,
> 
> Am 02.05.2018 um 14:57 schrieb Rene Pöschl:
>> Most of our contributors have no previous experience with git. So we 
>> really can not expect them to understand anything beyond the github 
>> workflow. (They are experts in electronics. Not IT) I also don't believe 
>> we should burden the contributors with too much with this.
> 
> that's unfortunately true for most of contributors or contributions no
> matter what kind of software or project and that's a social challenge we
> all need to deal with.
> 
>> This is even true for the maintainers. They are selected because they 
>> understand how to make good symbols and footprints. (Understanding of 
>> industry standards, limitations of manufacturing processes and the 
>> limitations of kicad)
> 
> Reading this thread and following from time to time some library issue
> on GitHub (only by reading) I see this point in another light. It's up
> to the maintainers to get fulfilled the requirements of the project if
> he is accepting a pull request.
> 
> I see long lists of pull request on GitHub for the libraries which, and
> this list seems even to increase more over the last months which is a
> good sign I think, which waiting a review from one of the people which
> are allowed to do merge requests. OTOH I see also commit messages as
> Reece has pointed out. So for me it looks like there are two problems
> which interacting together.
> 
> 1. The maintainers (you mean above I assume), which are needed to make
> QS on new or modified parts based on the library standards, are also
> "only" volunteering by there free time and this brings long waiting
> queues even for simply contributions. This shows a lack of available
> human resources with a deep understanding what the given standards are
> mean on technical work.
> 
> 2. There are also some library maintaining people needed which know how
> to do the technical git work right. And this means mostly you sometimes
> have to leave the world of platforms like GitHub, GitLab etc if needed.
> If you know what a git merge really is and how a GitHub PR is
> technically working under the hood you are able to solve quickly
> contributions which otherwise need to wait until someone with a better
> technical understanding will provide something useful in the issue
> tracker on GitHub. Over the time this can become a serious problem for a
> project as people can get quickly frustrated and wont contributing in
> future times.
> 
>> For a lot of our casual contributors the current way of contributing is 
>> already too complex. I have seen a lot of them simply give up as soon as 
>> git did something they did not expect. This is especially common on the 
>> symbol lib side as it is likely that the contributor needs to do a 
>> manual merge because somebody else touched the same lib.
> 
> Also this problem isn't new and is related to git can't handle big
> rather complex text files well if two persons have changed such a file.
> git was developed for source code not for handling large text based
> files. In the end you will come to the conclusion that git isn't the
> best tool here but it's also the best we have.
> For me than the workflow heavily based direct on plain git isn't the
> correct one.
> 
> Thinking in long term where more KiCad users will hopefully come the
> current model of library modifications will not work well enough and
> better forms of possibilities for contributions need to prepared. The
> GitHub platform is only one part to get all things together.
> 
>> These casual contributions can in most cases be summarized as a single 
>> "added symbol for component x" message. The enduser might not be 
>> concerned on the detailed mistakes made by the contributor. (And if they 
>> are, the pull request is always linked to the contribution anyways.)
>> As we don't want users to hide their history from us maintainers (we 
>> want to see what they changed after feedback from us) we really do not 
>> want them to squash their commits (by using amend).
> 
> This information is quite useless in the end and confusing due blowing
> of commits and meta information by this. You need this information only
> to decide if further changes are needed before merging or committing.
> For me later all these changes are pointless. So please no merge commits
> in pull requests and no further modification commits.
> 
> For classical pull requests on mailing list this information is handled
> in the 

Re: [Kicad-developers] A reminder on Git commit comments

2018-05-03 Thread Carsten Schoenert
Hello Rene,

Am 02.05.2018 um 14:57 schrieb Rene Pöschl:
> Most of our contributors have no previous experience with git. So we 
> really can not expect them to understand anything beyond the github 
> workflow. (They are experts in electronics. Not IT) I also don't believe 
> we should burden the contributors with too much with this.

that's unfortunately true for most of contributors or contributions no
matter what kind of software or project and that's a social challenge we
all need to deal with.

> This is even true for the maintainers. They are selected because they 
> understand how to make good symbols and footprints. (Understanding of 
> industry standards, limitations of manufacturing processes and the 
> limitations of kicad)

Reading this thread and following from time to time some library issue
on GitHub (only by reading) I see this point in another light. It's up
to the maintainers to get fulfilled the requirements of the project if
he is accepting a pull request.

I see long lists of pull request on GitHub for the libraries which, and
this list seems even to increase more over the last months which is a
good sign I think, which waiting a review from one of the people which
are allowed to do merge requests. OTOH I see also commit messages as
Reece has pointed out. So for me it looks like there are two problems
which interacting together.

1. The maintainers (you mean above I assume), which are needed to make
QS on new or modified parts based on the library standards, are also
"only" volunteering by there free time and this brings long waiting
queues even for simply contributions. This shows a lack of available
human resources with a deep understanding what the given standards are
mean on technical work.

2. There are also some library maintaining people needed which know how
to do the technical git work right. And this means mostly you sometimes
have to leave the world of platforms like GitHub, GitLab etc if needed.
If you know what a git merge really is and how a GitHub PR is
technically working under the hood you are able to solve quickly
contributions which otherwise need to wait until someone with a better
technical understanding will provide something useful in the issue
tracker on GitHub. Over the time this can become a serious problem for a
project as people can get quickly frustrated and wont contributing in
future times.

> For a lot of our casual contributors the current way of contributing is 
> already too complex. I have seen a lot of them simply give up as soon as 
> git did something they did not expect. This is especially common on the 
> symbol lib side as it is likely that the contributor needs to do a 
> manual merge because somebody else touched the same lib.

Also this problem isn't new and is related to git can't handle big
rather complex text files well if two persons have changed such a file.
git was developed for source code not for handling large text based
files. In the end you will come to the conclusion that git isn't the
best tool here but it's also the best we have.
For me than the workflow heavily based direct on plain git isn't the
correct one.

Thinking in long term where more KiCad users will hopefully come the
current model of library modifications will not work well enough and
better forms of possibilities for contributions need to prepared. The
GitHub platform is only one part to get all things together.

> These casual contributions can in most cases be summarized as a single 
> "added symbol for component x" message. The enduser might not be 
> concerned on the detailed mistakes made by the contributor. (And if they 
> are, the pull request is always linked to the contribution anyways.)
> As we don't want users to hide their history from us maintainers (we 
> want to see what they changed after feedback from us) we really do not 
> want them to squash their commits (by using amend).

This information is quite useless in the end and confusing due blowing
of commits and meta information by this. You need this information only
to decide if further changes are needed before merging or committing.
For me later all these changes are pointless. So please no merge commits
in pull requests and no further modification commits.

For classical pull requests on mailing list this information is handled
in the patch itself as this text block which will not result in the
final git history. Looks like this

> --
> Changes
> v1 -> v2
>  - With drop of patch to remove cflags use of relro flags, needed
>to rework variable assignments for use of spec files (Matt W.)
> ---

http://patchwork.ozlabs.org/patch/907093/

On GitHub you have tracked such information already within the issue
itself. And if you add a "Closes: #1234" into your commit message the
WebUI on GitHub will point you to the associated issue.

So no, this is all not needed and only time consuming on both sides.

> So the best solution might be to use the github squash merge feature 
> instead of a normal 

Re: [Kicad-developers] A reminder on Git commit comments

2018-05-02 Thread Rene Pöschl
Most of our contributors have no previous experience with git. So we 
really can not expect them to understand anything beyond the github 
workflow. (They are experts in electronics. Not IT) I also don't believe 
we should burden the contributors with too much with this.


This is even true for the maintainers. They are selected because they 
understand how to make good symbols and footprints. (Understanding of 
industry standards, limitations of manufacturing processes and the 
limitations of kicad)


For a lot of our casual contributors the current way of contributing is 
already too complex. I have seen a lot of them simply give up as soon as 
git did something they did not expect. This is especially common on the 
symbol lib side as it is likely that the contributor needs to do a 
manual merge because somebody else touched the same lib.


These casual contributions can in most cases be summarized as a single 
"added symbol for component x" message. The enduser might not be 
concerned on the detailed mistakes made by the contributor. (And if they 
are, the pull request is always linked to the contribution anyways.)
As we don't want users to hide their history from us maintainers (we 
want to see what they changed after feedback from us) we really do not 
want them to squash their commits (by using amend).


So the best solution might be to use the github squash merge feature 
instead of a normal merge and require the maintainer to write a good 
message about what changed. (Of course only if the contributor did not 
already make good commit messages.)



On 02/05/18 14:06, Wayne Stambaugh wrote:

This is why we have a commit message policy.  Obviously this isn't a
perfect solution.  Feel free to adopt this policy for library developer
commits.  There is always the hard line approach where you kick the
merge request back to the submitted and ask them to fix the commit
message or use `git commit --amend` to fix the commit message yourself.

On 5/1/2018 6:03 PM, Reece R. Pollack wrote:

I'd like to make an observation on some of the Git commit comments I'm
reading. Some of these commit log entries are really unhelpful.

When I was at University many, many years ago, students were taught to
place comments in their code. Many misunderstood the purpose of
comments, though, and would write code like this:

 x = x + 1;   /* Add 1 to x */


While the comment is totally correct, it's also totally useless. I can
/see/ that this line adds 1 to /x/; I don't need a comment to tell me
that. As a programmer coming along later, what I need to know is /why/ 1
is being added to /x/.

I'm seeing commit comments of the form "add one to x" in the KiCad
repos. These comments are useless. For an example, look at this commit
in the kicad-symbols repo:

 commit 4ada78e26774c841ce8ec33e8b221d04fed1f4c7
 Author: Rene Pöschl 
 Date:   Fri Apr 20 11:14:47 2018 +0200

 Remove Connector_Specialized files.

Great. I can /see/ that these files are being removed. That's obvious
from looking at the content of the commit. But there is no explanation
of why these files were deleted. Don't tell me that there was discussion
about this in email; even if that happened it's irrelevant. No one
should have to go chasing through an email archive hoping to understand
why a change was made. That's what the commit log entry is for.

I went back further in the Git logs for commits touching these files
hoping to find a more illuminating commit comment, but found nothing
helpful. In fact, I came across an earlier series of commits that are
even /less/ helpful:

 commit 467170586b106d00c4df185dc1683046e259b74c
 Author: evanshultz 
 Date:   Thu Apr 19 09:42:52 2018 -0700

 Just checking... still no dice

 commit 966a1a40aa36732d9c2404b044a4ef63bd0bb417
 Author: evanshultz 
 Date:   Thu Apr 19 09:36:28 2018 -0700

 Delete Connector_Specialized.lib

 commit aeb4270213da8a063f24163d6fa31e8521f08a83
 Author: evanshultz 
 Date:   Thu Apr 19 09:36:03 2018 -0700

 Delete Connector_Specialized.dcm

The top (latest) commit actually reverts the actions of the previous two
commits. So now I have the idea that there's some sort of disagreement
whether these files should be part of the repo or not. But I still have
no idea why.

In a previous job I was the poor sod (or arrogant bastard, take your
pick) who had to review commits before they were merged into the master
branch. I would have rejected commits like these immediately. With Open
Source development there's often less control over what gets merged and
what doesn't. But that just means it's all the more important for
everyone to make an effort to write good commit comments.

 From the KiCad source file
Documentation/development/commit-message-format.md:

 Commit messages should begin 

Re: [Kicad-developers] A reminder on Git commit comments

2018-05-02 Thread Wayne Stambaugh
This is why we have a commit message policy.  Obviously this isn't a
perfect solution.  Feel free to adopt this policy for library developer
commits.  There is always the hard line approach where you kick the
merge request back to the submitted and ask them to fix the commit
message or use `git commit --amend` to fix the commit message yourself.

On 5/1/2018 6:03 PM, Reece R. Pollack wrote:
> I'd like to make an observation on some of the Git commit comments I'm
> reading. Some of these commit log entries are really unhelpful.
> 
> When I was at University many, many years ago, students were taught to
> place comments in their code. Many misunderstood the purpose of
> comments, though, and would write code like this:
> 
> x = x + 1;   /* Add 1 to x */
> 
> 
> While the comment is totally correct, it's also totally useless. I can
> /see/ that this line adds 1 to /x/; I don't need a comment to tell me
> that. As a programmer coming along later, what I need to know is /why/ 1
> is being added to /x/.
> 
> I'm seeing commit comments of the form "add one to x" in the KiCad
> repos. These comments are useless. For an example, look at this commit
> in the kicad-symbols repo:
> 
> commit 4ada78e26774c841ce8ec33e8b221d04fed1f4c7
> Author: Rene Pöschl 
> Date:   Fri Apr 20 11:14:47 2018 +0200
> 
>     Remove Connector_Specialized files.
> 
> Great. I can /see/ that these files are being removed. That's obvious
> from looking at the content of the commit. But there is no explanation
> of why these files were deleted. Don't tell me that there was discussion
> about this in email; even if that happened it's irrelevant. No one
> should have to go chasing through an email archive hoping to understand
> why a change was made. That's what the commit log entry is for.
> 
> I went back further in the Git logs for commits touching these files
> hoping to find a more illuminating commit comment, but found nothing
> helpful. In fact, I came across an earlier series of commits that are
> even /less/ helpful:
> 
> commit 467170586b106d00c4df185dc1683046e259b74c
> Author: evanshultz 
> Date:   Thu Apr 19 09:42:52 2018 -0700
> 
>     Just checking... still no dice
> 
> commit 966a1a40aa36732d9c2404b044a4ef63bd0bb417
> Author: evanshultz 
> Date:   Thu Apr 19 09:36:28 2018 -0700
> 
>     Delete Connector_Specialized.lib
> 
> commit aeb4270213da8a063f24163d6fa31e8521f08a83
> Author: evanshultz 
> Date:   Thu Apr 19 09:36:03 2018 -0700
> 
>     Delete Connector_Specialized.dcm
> 
> The top (latest) commit actually reverts the actions of the previous two
> commits. So now I have the idea that there's some sort of disagreement
> whether these files should be part of the repo or not. But I still have
> no idea why.
> 
> In a previous job I was the poor sod (or arrogant bastard, take your
> pick) who had to review commits before they were merged into the master
> branch. I would have rejected commits like these immediately. With Open
> Source development there's often less control over what gets merged and
> what doesn't. But that just means it's all the more important for
> everyone to make an effort to write good commit comments.
> 
> From the KiCad source file
> Documentation/development/commit-message-format.md:
> 
> Commit messages should begin with a subject line; try to limit this
> to no more
> than 50-72 characters. The body of the message should be separated
> from the
> subject by a blank line and wrapped at 72 characters. The body of a
> commit
> message should explain what the commit does and why, but do not
> explain *how*
> as the code itself should do that.
> 
> 
> 
> 
> ___
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
> 

___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp


Re: [Kicad-developers] A reminder on Git commit comments

2018-05-02 Thread Rene Pöschl

Thanks for the reminder,
we will try to better our ways.

See: https://github.com/KiCad/kicad-symbols/issues/569

On 02/05/18 00:03, Reece R. Pollack wrote:
I'd like to make an observation on some of the Git commit comments I'm 
reading. Some of these commit log entries are really unhelpful.


When I was at University many, many years ago, students were taught to 
place comments in their code. Many misunderstood the purpose of 
comments, though, and would write code like this:


x = x + 1;   /* Add 1 to x */


While the comment is totally correct, it's also totally useless. I can 
/see/ that this line adds 1 to /x/; I don't need a comment to tell me 
that. As a programmer coming along later, what I need to know is /why/ 
1 is being added to /x/.


I'm seeing commit comments of the form "add one to x" in the KiCad 
repos. These comments are useless. For an example, look at this commit 
in the kicad-symbols repo:


commit 4ada78e26774c841ce8ec33e8b221d04fed1f4c7
Author: Rene Pöschl 
Date:   Fri Apr 20 11:14:47 2018 +0200

    Remove Connector_Specialized files.

Great. I can /see/ that these files are being removed. That's obvious 
from looking at the content of the commit. But there is no explanation 
of why these files were deleted. Don't tell me that there was 
discussion about this in email; even if that happened it's irrelevant. 
No one should have to go chasing through an email archive hoping to 
understand why a change was made. That's what the commit log entry is for.


I went back further in the Git logs for commits touching these files 
hoping to find a more illuminating commit comment, but found nothing 
helpful. In fact, I came across an earlier series of commits that are 
even /less/ helpful:


commit 467170586b106d00c4df185dc1683046e259b74c
Author: evanshultz 
Date:   Thu Apr 19 09:42:52 2018 -0700

    Just checking... still no dice

commit 966a1a40aa36732d9c2404b044a4ef63bd0bb417
Author: evanshultz 
Date:   Thu Apr 19 09:36:28 2018 -0700

    Delete Connector_Specialized.lib

commit aeb4270213da8a063f24163d6fa31e8521f08a83
Author: evanshultz 
Date:   Thu Apr 19 09:36:03 2018 -0700

    Delete Connector_Specialized.dcm

The top (latest) commit actually reverts the actions of the previous 
two commits. So now I have the idea that there's some sort of 
disagreement whether these files should be part of the repo or not. 
But I still have no idea why.


In a previous job I was the poor sod (or arrogant bastard, take your 
pick) who had to review commits before they were merged into the 
master branch. I would have rejected commits like these immediately. 
With Open Source development there's often less control over what gets 
merged and what doesn't. But that just means it's all the more 
important for everyone to make an effort to write good commit comments.


From the KiCad source file 
Documentation/development/commit-message-format.md:


Commit messages should begin with a subject line; try to limit
this to no more
than 50-72 characters. The body of the message should be separated
from the
subject by a blank line and wrapped at 72 characters. The body of
a commit
message should explain what the commit does and why, but do not
explain *how*
as the code itself should do that.




___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp




___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp


Re: [Kicad-developers] A reminder on Git commit comments

2018-05-01 Thread Adam Wolf
I believe most of the contributors to kicad-symbols actually do not read
this mailing list. I know Rene does, but this may be better addressed by
watching PRs on the kicad-symbols GitHub repository and helping
contributors better phrase their commit messages.

Adam Wolf

On Tue, May 1, 2018, 5:04 PM Reece R. Pollack  wrote:

> I'd like to make an observation on some of the Git commit comments I'm
> reading. Some of these commit log entries are really unhelpful.
>
> When I was at University many, many years ago, students were taught to
> place comments in their code. Many misunderstood the purpose of comments,
> though, and would write code like this:
>
> x = x + 1;   /* Add 1 to x */
>
>
> While the comment is totally correct, it's also totally useless. I can
> *see* that this line adds 1 to *x*; I don't need a comment to tell me
> that. As a programmer coming along later, what I need to know is *why* 1
> is being added to *x*.
>
> I'm seeing commit comments of the form "add one to x" in the KiCad repos.
> These comments are useless. For an example, look at this commit in the
> kicad-symbols repo:
>
> commit 4ada78e26774c841ce8ec33e8b221d04fed1f4c7
> Author: Rene Pöschl 
> 
> Date:   Fri Apr 20 11:14:47 2018 +0200
>
> Remove Connector_Specialized files.
>
> Great. I can *see* that these files are being removed. That's obvious
> from looking at the content of the commit. But there is no explanation of
> why these files were deleted. Don't tell me that there was discussion about
> this in email; even if that happened it's irrelevant. No one should have to
> go chasing through an email archive hoping to understand why a change was
> made. That's what the commit log entry is for.
>
> I went back further in the Git logs for commits touching these files
> hoping to find a more illuminating commit comment, but found nothing
> helpful. In fact, I came across an earlier series of commits that are even
> *less* helpful:
>
> commit 467170586b106d00c4df185dc1683046e259b74c
> Author: evanshultz 
> 
> Date:   Thu Apr 19 09:42:52 2018 -0700
>
> Just checking... still no dice
>
> commit 966a1a40aa36732d9c2404b044a4ef63bd0bb417
> Author: evanshultz 
> 
> Date:   Thu Apr 19 09:36:28 2018 -0700
>
> Delete Connector_Specialized.lib
>
> commit aeb4270213da8a063f24163d6fa31e8521f08a83
> Author: evanshultz 
> 
> Date:   Thu Apr 19 09:36:03 2018 -0700
>
> Delete Connector_Specialized.dcm
>
> The top (latest) commit actually reverts the actions of the previous two
> commits. So now I have the idea that there's some sort of disagreement
> whether these files should be part of the repo or not. But I still have no
> idea why.
>
> In a previous job I was the poor sod (or arrogant bastard, take your pick)
> who had to review commits before they were merged into the master branch. I
> would have rejected commits like these immediately. With Open Source
> development there's often less control over what gets merged and what
> doesn't. But that just means it's all the more important for everyone to
> make an effort to write good commit comments.
>
> From the KiCad source file
> Documentation/development/commit-message-format.md:
>
> Commit messages should begin with a subject line; try to limit this to no
> more
> than 50-72 characters. The body of the message should be separated from the
> subject by a blank line and wrapped at 72 characters. The body of a commit
> message should explain what the commit does and why, but do not explain
> *how*
> as the code itself should do that.
>
>
> ___
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>
___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp


[Kicad-developers] A reminder on Git commit comments

2018-05-01 Thread Reece R. Pollack
I'd like to make an observation on some of the Git commit comments I'm 
reading. Some of these commit log entries are really unhelpful.


When I was at University many, many years ago, students were taught to 
place comments in their code. Many misunderstood the purpose of 
comments, though, and would write code like this:


   x = x + 1;   /* Add 1 to x */


While the comment is totally correct, it's also totally useless. I can 
/see/ that this line adds 1 to /x/; I don't need a comment to tell me 
that. As a programmer coming along later, what I need to know is /why/ 1 
is being added to /x/.


I'm seeing commit comments of the form "add one to x" in the KiCad 
repos. These comments are useless. For an example, look at this commit 
in the kicad-symbols repo:


   commit 4ada78e26774c841ce8ec33e8b221d04fed1f4c7
   Author: Rene Pöschl 
   Date:   Fri Apr 20 11:14:47 2018 +0200

    Remove Connector_Specialized files.

Great. I can /see/ that these files are being removed. That's obvious 
from looking at the content of the commit. But there is no explanation 
of why these files were deleted. Don't tell me that there was discussion 
about this in email; even if that happened it's irrelevant. No one 
should have to go chasing through an email archive hoping to understand 
why a change was made. That's what the commit log entry is for.


I went back further in the Git logs for commits touching these files 
hoping to find a more illuminating commit comment, but found nothing 
helpful. In fact, I came across an earlier series of commits that are 
even /less/ helpful:


   commit 467170586b106d00c4df185dc1683046e259b74c
   Author: evanshultz 
   Date:   Thu Apr 19 09:42:52 2018 -0700

    Just checking... still no dice

   commit 966a1a40aa36732d9c2404b044a4ef63bd0bb417
   Author: evanshultz 
   Date:   Thu Apr 19 09:36:28 2018 -0700

    Delete Connector_Specialized.lib

   commit aeb4270213da8a063f24163d6fa31e8521f08a83
   Author: evanshultz 
   Date:   Thu Apr 19 09:36:03 2018 -0700

    Delete Connector_Specialized.dcm

The top (latest) commit actually reverts the actions of the previous two 
commits. So now I have the idea that there's some sort of disagreement 
whether these files should be part of the repo or not. But I still have 
no idea why.


In a previous job I was the poor sod (or arrogant bastard, take your 
pick) who had to review commits before they were merged into the master 
branch. I would have rejected commits like these immediately. With Open 
Source development there's often less control over what gets merged and 
what doesn't. But that just means it's all the more important for 
everyone to make an effort to write good commit comments.


From the KiCad source file 
Documentation/development/commit-message-format.md:


   Commit messages should begin with a subject line; try to limit this
   to no more
   than 50-72 characters. The body of the message should be separated
   from the
   subject by a blank line and wrapped at 72 characters. The body of a
   commit
   message should explain what the commit does and why, but do not
   explain *how*
   as the code itself should do that.


___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp