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

Introduce LAST option in INSTALL and MOVE and change default behaviour #1113

Merged
merged 5 commits into from
Aug 15, 2022

Conversation

rdemaria
Copy link
Contributor

Addressing #1055
Introduce a new option in install and move command "LAST" that controls how an element is place with respect to other elements at the same location.

It changes the default behaviour! Before, a list of install commands at the s will install the element in the reversed order, and now it will respect the command order.

New default is particularly useful for aperture markers that can be place at the same location in case of sharp edges.

@rdemaria rdemaria requested review from tpersson and ldeniau May 24, 2022 12:39
@rdemaria rdemaria changed the title Stabilize order Introduce LAST option in INSTALL and MOVE and change default behaviour May 24, 2022
@rdemaria rdemaria modified the milestones: 5.08, 5.09 May 24, 2022
Copy link
Contributor

@tpersson tpersson left a comment

Choose a reason for hiding this comment

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

It is a useful functionality but why do we change the default behaviour? It might cause problems for some users and anyone who wants to change the default behaviour can do so after this PR. Anyway, I don't feel very strongly about it so go ahead if you think there is a good reason to change the default behaviour.

@rdemaria
Copy link
Contributor Author

The reason for the change is that often it is more intuitive and practical to install markers in the same order of the input file. This is the behaviour of the sequence file. Now it is the other way around.

I think the benefit of changing the default behaviour outweighs the backward incompatibility in this case.

@rdemaria rdemaria merged commit 3364035 into MethodicalAcceleratorDesign:master Aug 15, 2022
@rdemaria rdemaria deleted the stabilize_order branch August 15, 2022 12:05
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.

2 participants