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

Inconsistent order of operations #232

Open
LecrisUT opened this issue Apr 17, 2024 · 6 comments
Open

Inconsistent order of operations #232

LecrisUT opened this issue Apr 17, 2024 · 6 comments
Milestone

Comments

@LecrisUT
Copy link
Contributor

Example:

my_var:
  - one
  - two

/overwrite:
  my_var-: [ two, three ]
  my_var+: [ three, four ]

This one gives my_var: one and four which would be inconsistent based on the order of definition. This might not be that crucial, but at least it should be documented that the keys are ordered before being parsed by fmf, thus the order is ...

Are there tests at least covering the operation order based on inheritance (inherited operations occur first, e.g. if my_var-: was one level below, three should be in the list) and adjust (after base variable operations)?

@lukaszachy
Copy link
Collaborator

@LecrisUT
Copy link
Contributor Author

Oh, thanks for the info. Than the only other part of the issue is if there are tests that cover the plain order, the order with inheritance and the order with adjust

@lukaszachy
Copy link
Collaborator

Ack, coverage should be added as mistakes in refactor might happen.

@lukaszachy lukaszachy added this to the 1.4 milestone Apr 18, 2024
@lukaszachy lukaszachy self-assigned this Apr 18, 2024
@psss
Copy link
Collaborator

psss commented May 13, 2024

We need to document - implementation sorts keys - https://github.com/teemtee/fmf/blob/29190c78e4065adf98cbb3ccf747b01973d761e2/fmf/base.py#L222C1-L222C50

Hm, just pondering, is that sorted() call really intentional? I looked through the history and it appears in 51bf8de which is the very first fmf commit ;-) I'm not really sure we need to keep it. On the other hand, if removed (which could fix the problem reported here), can we rely on the order of the dict items?

@psss
Copy link
Collaborator

psss commented May 14, 2024

Decision from the hacking session: Respect the order of the adjustments and drop the sorted() from the code. Document the behavior.

@LecrisUT
Copy link
Contributor Author

LecrisUT commented May 14, 2024

Could break backwards compatibility, but I think this is quite a niche usage, we could just scrape the public .fmf files and announce the change there.

Edit: as far as I see we're safe: https://sourcegraph.com/search?q=context:global+file:.*%5C.fmf+%28%5Cw%2B%29%28-%7C%5C%2B%3C%29:&patternType=regexp&sm=0

@lukaszachy lukaszachy modified the milestones: 1.4, 1.5 May 28, 2024
@lukaszachy lukaszachy removed their assignment May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants