Skip to content

Conversation

@cgwalters
Copy link
Member

We already have fallback defaults in the code. Avoiding global
environment variables is good on general principle; but this
is specifically prep for making the entrypoint a Go program.

cgwalters added 2 commits July 5, 2022 17:52
Prep for not setting `COSA_META_SCHEMA` by default.
We already have fallback defaults in the code.  Avoiding global
environment variables is good on general principle; but this
is specifically prep for making the entrypoint a Go program.
@cgwalters cgwalters force-pushed the drop-set-cosa-meta-schema branch from 0d226d6 to 3d9f179 Compare July 5, 2022 21:52
@cgwalters
Copy link
Member Author

coreos-assembler in a nutshell: The Python code (which duplicates the Go code handling schemas) had a bug that was papered over by some messy shell script.

Comment on lines -77 to -82
COSA_META_SCHEMA="${COSA_META_SCHEMA:-/usr/lib/coreos-assembler/v1.json}"
schema_override="${PWD}/src/config/schema.json"
if [ -e "${schema_override}" ]; then
COSA_META_SCHEMA=$(realpath "${schema_override}")
fi
export COSA_META_SCHEMA
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so the code itself already behaves this way and there was no reason to do this?

Does it handle if "${PWD}/src/config/schema.json" exists?

I'm happy to delete this code, just don't know if anything is relying on that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it handle if "${PWD}/src/config/schema.json" exists?

Ah right, I should have commented on that. I don't believe that was ever actually used, so it should be safe to delete.

Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trivial LGTM

@cgwalters cgwalters merged commit 301790a into coreos:main Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants