← Back to team overview

kicad-developers team mailing list archive

Re: A reminder on Git commit comments

 

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

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

I think we speak a different language. I have no idea what you wrote above.

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

Our typical contribution does not reference an issue. After all most contributions are new "features" And even if we fix something. It is most likely not previously reported in an issue but was found by the librarian shortly before committing a fix. And even if there is an issue. Most of the time more than one pull request is needed to really fix all facets of this issue. (we could divide issues more, but then we take more time writing down issues then it would take to fix them.)

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.)
Yes, at least the maintainers need to do more like this or locally on
their machine before doing ping pong with pull or merge request. Don't
get me wrong, it's awesome what the libraries have been evolved over the
last year. It's just a never ending task as you will find always new
advantages to get things improved.

Our focus will always be on the quality of the result. Not necessarily on documenting how we get there.



Follow ups

References