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

SALTO-5643: Adding members in the order they are defined #5877

Merged
merged 7 commits into from
May 22, 2024

Conversation

yelly
Copy link
Contributor

@yelly yelly commented May 8, 2024

Instead of adding new fields, annotations or values at the end of their parent in NaCl files, add them in the order they are defined in their parent object. This will ensure that members always show up in the same place in the NaCl if they are removed and added again.


Additional context for reviewer
Still need to add higher level tests, IMO it's cleaner to split into a separate PR.


Release Notes:
Core

  • Element members (field, annotations or values) are added in the same location consistently instead of being appended.

User Notifications:
None.

@yelly yelly requested a review from ori-moisis May 8, 2024 15:24
@coveralls
Copy link

coveralls commented May 8, 2024

Coverage Status

coverage: 93.912% (-0.01%) from 93.923%
when pulling 4a28c0d on yelly:SALTO-5643
into 2d1d46b on salto-io:main.

Copy link
Contributor

@ori-moisis ori-moisis left a comment

Choose a reason for hiding this comment

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

didn't finish reviewing the logic, but I think there are enough comments to do a round...

@yelly yelly requested a review from ori-moisis May 12, 2024 07:50
@yelly yelly force-pushed the SALTO-5643 branch 3 times, most recently from 8c22d70 to 37510ae Compare May 15, 2024 11:27
@yelly yelly requested a review from ori-moisis May 16, 2024 09:08
@yelly yelly enabled auto-merge (squash) May 22, 2024 11:05
@yelly yelly merged commit 41b619a into salto-io:main May 22, 2024
19 checks passed
@yelly yelly deleted the SALTO-5643 branch May 22, 2024 11:48
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.

None yet

3 participants