Skip to content
New issue

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

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

Already on GitHub? # to your account

fixer: Address rename conflicts with explanation #1120

Merged
merged 3 commits into from
Sep 23, 2024

Conversation

charlieegan3
Copy link
Member

Fixes #1119

@charlieegan3 charlieegan3 force-pushed the fix-conflicts branch 2 times, most recently from 57a9aa7 to cf527d5 Compare September 19, 2024 14:11
Copy link
Member

@anderseknert anderseknert left a comment

Choose a reason for hiding this comment

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

LGTM! I guess one option for later could be to simply create foo1.rego, foo2.rego, and so on, in case of conflicts, instead of failing.. at least if configured to do so?

return fmt.Errorf("failed to create reporter for format %s: %w", params.format, err)
}

r.SetDryRun(params.dryRun)
Copy link
Member

Choose a reason for hiding this comment

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

Should this not be set any more?

e2e/cli_test.go Outdated

import rego.v1
`,
// these thre files should all be at bar/bar.rego, but they cannot all be moved there
Copy link
Member

Choose a reason for hiding this comment

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

thre -> three / 3

@@ -11,6 +11,15 @@ import (
"github.com/styrainc/regal/pkg/rules"
)

type RenameConflictError struct {
Copy link
Member

Choose a reason for hiding this comment

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

Nice! We should do more of this.

Fixes StyraInc#1119

Signed-off-by: Charlie Egan <charlie@styra.com>
Now conflicts are reported by type. If they conflict with an existing
file, these are shown first. Then, conflicts created by more than one
file being moved to the same location are shown after that - if any.

Signed-off-by: Charlie Egan <charlie@styra.com>
Signed-off-by: Charlie Egan <charlie@styra.com>
@charlieegan3 charlieegan3 merged commit a23bb63 into StyraInc:main Sep 23, 2024
4 checks passed
@charlieegan3 charlieegan3 deleted the fix-conflicts branch September 23, 2024 12:16
srenatus pushed a commit to srenatus/regal that referenced this pull request Oct 1, 2024
* fixer: Address rename conflicts with explanation

Fixes StyraInc#1119

Signed-off-by: Charlie Egan <charlie@styra.com>

* fixer: Output the source of conflicts in report

Now conflicts are reported by type. If they conflict with an existing
file, these are shown first. Then, conflicts created by more than one
file being moved to the same location are shown after that - if any.

Signed-off-by: Charlie Egan <charlie@styra.com>

* fix: Address PR comments

Signed-off-by: Charlie Egan <charlie@styra.com>

---------

Signed-off-by: Charlie Egan <charlie@styra.com>
charlieegan3 added a commit to charlieegan3/regal that referenced this pull request Jan 6, 2025
* fixer: Address rename conflicts with explanation

Fixes StyraInc#1119

Signed-off-by: Charlie Egan <charlie@styra.com>

* fixer: Output the source of conflicts in report

Now conflicts are reported by type. If they conflict with an existing
file, these are shown first. Then, conflicts created by more than one
file being moved to the same location are shown after that - if any.

Signed-off-by: Charlie Egan <charlie@styra.com>

* fix: Address PR comments

Signed-off-by: Charlie Egan <charlie@styra.com>

---------

Signed-off-by: Charlie Egan <charlie@styra.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix: Output a better error when attempting to rename files to the same destination
2 participants