Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: use otk files instead of otk.arguments #7

Merged
merged 1 commit into from
Apr 15, 2024

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Apr 10, 2024

The use-case given (and maybe I'm missing other use-cases here) for the otk.arguments is currently "version" and "architecture".

We could do this instead via a fedora-39-amd64.yaml file that just define those arguments. The upside of this is that we can easily see what combinations of version and architecture we support by looking at the top-level dir. It also simplifies the spec and we would not have to worry about validating arguments, i.e. how do we know that "-Dversion=41" is valid or not (if not valid it would probably mean down the line an include based on the version cannot be found and that presents challenges for a nice error message).

It would also allow us to have symlinks when things do not change, e.g.

rhel-9.3.yaml -> rhel-9.4.yaml

But maybe I'm missing another use case for this? Even then I think the fedora/39-amd64.yaml has some merrits and is worth considering.

@supakeen
Copy link
Member

supakeen commented Apr 11, 2024

Let's drop the otk.argument directive until there's a clear usecase for it. Can you update this PR?

The use-case given (and maybe I'm missing other use-cases here) for
the otk.arguments is currently "version" and "architecture".

We could do this instead via a `fedora-39-amd64.yaml` file that
just define those arguments. The upside of this is that we can
easily see what combinations of version and architecture we
support by looking at the top-level dir. It also simplifies the
spec and we would not have to worry about validating arguments,
i.e. how do we know that "-Dversion=41" is valid or not (if not
valid it would probably mean down the line an include based on
the version cannot be found and that presents challenges for a
nice error message).

It would also allow us to have symlinks when things do not change,
e.g.
```
rhel-9.3.yaml -> rhel-9.4.yaml
```

But maybe I'm missing another use case for this? Even then I think
the `fedora/39-amd64.yaml` has some merrits and is worth considering.
Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

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

I really like this simplification. It reduces the scope without limiting functionality.

@supakeen supakeen added this pull request to the merge queue Apr 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Apr 12, 2024
@supakeen supakeen added this pull request to the merge queue Apr 15, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Apr 15, 2024
@supakeen supakeen added this pull request to the merge queue Apr 15, 2024
@supakeen supakeen removed this pull request from the merge queue due to a manual request Apr 15, 2024
@supakeen supakeen merged commit 6e2206e into osbuild:main Apr 15, 2024
1 check passed
@mvo5 mvo5 deleted the no-arguments branch April 15, 2024 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants