Skip to content

Stable write logic - #136

Merged
rafaelfranca merged 1 commit into
mainfrom
stable-write-logic
Jul 29, 2025
Merged

Stable write logic#136
rafaelfranca merged 1 commit into
mainfrom
stable-write-logic

Conversation

@tjoyal

@tjoyal tjoyal commented Jul 29, 2025

Copy link
Copy Markdown
Member

TLDR: Introduce .sort.to_h on write.

A common issue with using the deprecation toolkit in large organizations is that new faults are appended to the end of the file, which in a collaborative environment, can cause unnecessary merge conflicts.

By implementing .sort.to_h, we can ensure that deprecations are organized in a predictable manner. This reduces merge conflicts by aligning the order of the deprecations with the order of the original test file.

Additionally, I've revised the write method to prevent unnecessary file system creation, updates, and deletions where not required.

Lastly, I've incorporated logic to prevent file updates when the original and updated content are logically equivalent. In the context of an existing project, reordering the YAML content with .sort.to_h can be disruptive if there are no significant changes. I've chosen not to update the file in such cases, but an alternative approach could be to enforce the new ordering regardless.

I'm unsure of the best way to test this proposal, but existing tests do validate the overall functionality.

@tjoyal
tjoyal force-pushed the stable-write-logic branch from 6870e72 to c2e03a9 Compare July 29, 2025 12:25
@tjoyal
tjoyal marked this pull request as ready for review July 29, 2025 12:34

@etiennebarrie etiennebarrie left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah I've been bitten by this, the approach to avoid unnecessary changes sounds reasonable.

@rafaelfranca
rafaelfranca merged commit eb6e762 into main Jul 29, 2025
39 checks passed
@rafaelfranca
rafaelfranca deleted the stable-write-logic branch July 29, 2025 17:59
@Edouard-chin

Edouard-chin commented Jul 29, 2025

Copy link
Copy Markdown
Member

It's been a very long time I didn't have a look at this gem. But if I remember correctly, the deprecations where written in the same order they were triggered purposedly.
This was to ensure that the next time a test runs, the same order of deprecation would be triggered, this was to catch flaky tests.

I believe that if we want to order them, we'd need to order them when they get triggered otherwise we'd have false positive because of

deprecations_without_stacktrace <=> other.deprecations_without_stacktrace

I'll check tomorrow when I have a bit of time between 2 onboarding sessions :)

@Edouard-chin

Copy link
Copy Markdown
Member

Ah, I missed that the ordering is done on the test names not on the actual deprecations array.
All good, thank you !

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.

4 participants