Skip to content

mantle: Remove generated cosa schema from git#1306

Closed
cgwalters wants to merge 1 commit intocoreos:masterfrom
cgwalters:always-build-schema
Closed

mantle: Remove generated cosa schema from git#1306
cgwalters wants to merge 1 commit intocoreos:masterfrom
cgwalters:always-build-schema

Conversation

@cgwalters
Copy link
Copy Markdown
Member

My recent change to use make for this seems to have introduced
a race condition mainly in Prow; perhaps something is not
preserving timestamps.

In the end anyways, files stored in git should not be touched
via a default make, as that is both subject to timestamp
problems and also creates friction.

Generating it is cheap, so let's switch to always generating it.

My recent change to use `make` for this seems to have introduced
a race condition mainly in Prow; perhaps something is not
preserving timestamps.

In the end anyways, files stored in git should not be touched
via a default `make`, as that is both subject to timestamp
problems and also creates friction.

Generating it is cheap, so let's switch to always generating it.
@openshift-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@cgwalters
Copy link
Copy Markdown
Member Author

cgwalters commented Mar 31, 2020

The other alternative is to store it in git and require a make schema-update or so. I generally lean towards not storing generated files in Git as a general rule, but then in the Go ecosystem vendor/ is popular which is the totally opposite thing.

@dustymabe
Copy link
Copy Markdown
Member

I have no opinion on this so I'll let others who do weigh in

@jlebon
Copy link
Copy Markdown
Member

jlebon commented Apr 1, 2020

Hmm, lukewarm on this, leaning more towards:

The other alternative is to store it in git and require a make schema-update or so.

The reason is that it's useful to be able to read the golang schema without having to build the code. And IMO reading the original JSON schema is way harder than the generated Go code. This also matches what Ignition does.

But anyway, not super strongly against it either if others find it valuable.

cgwalters added a commit to cgwalters/coreos-assembler that referenced this pull request Apr 1, 2020
Alternative to coreos#1306

My recent change to use make for this seems to have introduced
a race condition mainly in Prow; perhaps something is not
preserving timestamps.

Files stored in git should not be touched
via a default make, as that is both subject to timestamp
problems and also creates friction.

So this change now requires an explicit manual generation.
@cgwalters
Copy link
Copy Markdown
Member Author

ok then #1308

cgwalters added a commit to cgwalters/coreos-assembler that referenced this pull request Apr 1, 2020
Alternative to coreos#1306

My recent change to use make for this seems to have introduced
a race condition mainly in Prow; perhaps something is not
preserving timestamps.

Files stored in git should not be touched
via a default make, as that is both subject to timestamp
problems and also creates friction.

So this change now requires an explicit manual generation; and
we also consistently add the generated doc file to git too.
@cgwalters
Copy link
Copy Markdown
Member Author

Closing in favor of the other one

@cgwalters cgwalters closed this Apr 1, 2020
openshift-merge-robot pushed a commit that referenced this pull request Apr 2, 2020
Alternative to #1306

My recent change to use make for this seems to have introduced
a race condition mainly in Prow; perhaps something is not
preserving timestamps.

Files stored in git should not be touched
via a default make, as that is both subject to timestamp
problems and also creates friction.

So this change now requires an explicit manual generation; and
we also consistently add the generated doc file to git too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants