PCB shouldn't store file meta data inside the file

[Traumflug]

The current file format stores "date", "user" and "host" data inside the file (line 2...4). All of these infos can be either obtained by looking at the file's meta data provided by the file system or isn't interesting at all.

The reason to remove this is, it regularly causes merge conflicts when handling layout files with a versioning control system like SVN or Git. Removing meta data inside the file looses nothing and enhances handling layouts with VCSs a lot. Storing layouts in VCS repositories is common these days, especially in projects with collaborative development.

[Peter Clifton (pcjc2)]

I'd be happy to see them go, however obviously someone at some point wanted to add them..

One could imagine date and user being useful in some circumstances, and if you compare to office-software suites, you will probably find that they also store metadata like this along with their files.

If I get an agreement from DJ and / or Dan that this should go, I'll happily commit a patch which removes these.

Don't spend too much time working on a patch until we know if it is likely to get accepted though!

[DJ Delorie (djdelorie)]

Perhaps a flow that needs that information can put it in attributes, instead of comments that get discarded every time the file is read?

[Peter Clifton (pcjc2)]

From chatting to Dan on IRC, I think he is happy to see them go as well.

danmc: on the pcb file metadata, that code was there in feb 2003 when I imported pcb sources
danmc: I don't see a reason to keep it.
pcjc2: Sounds fine - so if Markus comes up with a patch, we can take it out
pcjc2: I just didn't want to be the person who unilaterally removed it
danmc: and I can see that some users may not want it there (leaks gecos info)

[Traumflug (mah-jump-ing)]

Here's the first part of the patch. As far as I can see, this information isn't even read back in, so there should be no side effects.

Missing parts:

- remove these three lines in all sample and test .pcb files (easy to accomplish)

- the test for HAVE_GETHOSTNAME in the autoconf system is now obsolete (never worked on this autoconf thing, so no idea on how to do this one)

[Traumflug (mah-jump-ing)]

Patch for removing these 3 lines from the sample files. Not really tested, as I don't have the setup to build documentation.

[Traumflug (mah-jump-ing)]

This patch removes the test for gethostname(), as it's obsolete now. I hope it's done right, at least it works nicely.

[Traumflug (mah-jump-ing)]

I'm done with coding on this feature.

[Peter Clifton (pcjc2)]

It isn't "Fix Committed" until someone has pushed the code into the git repository, so I'm changing back to "In Progress"

[Peter Clifton (pcjc2)]

I'll assign this bug to me now, to remind me to review and push the code. Probably won't be today unfortunately.

[Traumflug (mah-jump-ing)]

> It isn't "Fix Committed" until someone has pushed the code into the
> git
> repository

Probably there's a "Fix done" missing.

Anyways, thanks for taking care.

[Peter Clifton (pcjc2)]

> Probably there's a "Fix done" missing.

In the Canonical / Launchpad world (not our world), they use bzr branches which integrate well with Launchpad. For launchpad development, for example - they would push a branch and then request a merge with the main product repository - which starts a review process.

Other ways of dealing with this kind of work-flow issue would be to have a team called "geda-patch-review", which we would document as part of the work-flow, e.g. "When a patch is ready for review, subscribe the "geda-patch-review" team.

(Or "assign to the geda-patch-review" team). If we need to, we could create such a team and try workflows like that.

[DJ Delorie (djdelorie)]

We need a "state" that means "needs review" that lives between
assigned and committed. In a previous job, our source control system
had about 20 states from "created" through "approved", including
various stages of acceptance, coding, review, and testing. It also
allowed for multiple ownerships, from developer through QA and
release, depending what state it was in.

I wouldn't want to take assignment away from the user who did the
work, just to say "I'm done, please check it".

[Peter Clifton (pcjc2)]

Focus-team subscription is the way to do it I think.
Also - "assignment" is just used to mean - who is working on this currently (or who is blocking it)

Agreed it would be nice to have multiple assignment. I may bring this up next time I'm chatting to LP developers.

[Peter Clifton (pcjc2)]

Committed, thanks!