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

core, editoast: return conflict requirements #8758

Merged
merged 1 commit into from
Sep 17, 2024

Conversation

emersion
Copy link
Member

@emersion emersion commented Sep 4, 2024

Closes: #8680

There are two TODOs left which are open questions for reviewers.

@emersion emersion force-pushed the emr/core-return-conflict-reqs branch from 9193c19 to bb6ee07 Compare September 5, 2024 13:04
@emersion emersion marked this pull request as ready for review September 5, 2024 13:06
@emersion emersion requested review from a team as code owners September 5, 2024 13:06
@emersion emersion force-pushed the emr/core-return-conflict-reqs branch from bb6ee07 to c8b9439 Compare September 5, 2024 13:16
Copy link
Contributor

@woshilapin woshilapin left a comment

Choose a reason for hiding this comment

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

Looks good, I leave you in the expert hands of core maintainers for your questions.

flomonster

This comment was marked as off-topic.

@emersion emersion force-pushed the emr/core-return-conflict-reqs branch 2 times, most recently from a8228b7 to 5208c44 Compare September 16, 2024 10:11
@emersion emersion force-pushed the emr/core-return-conflict-reqs branch 2 times, most recently from 095b8a3 to 9f42af5 Compare September 17, 2024 11:37
@emersion
Copy link
Member Author

Is this the kind of test that you had in mind? I just kind of abused the first test here, but could easily move it to another test (maybe one with multiple conflicts? which one? I also wouldn't want to hardcode tons of expected output fields...).

@emersion emersion force-pushed the emr/core-return-conflict-reqs branch from 9f42af5 to d941c58 Compare September 17, 2024 11:43
Copy link
Contributor

@Erashin Erashin left a comment

Choose a reason for hiding this comment

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

Apart from the fix needed on the unit test, this looks good. Thanks :)

@emersion emersion force-pushed the emr/core-return-conflict-reqs branch from d941c58 to 018d1fa Compare September 17, 2024 13:53
@emersion emersion force-pushed the emr/core-return-conflict-reqs branch from 018d1fa to 97091e0 Compare September 17, 2024 14:01
@emersion emersion added this pull request to the merge queue Sep 17, 2024
Merged via the queue into dev with commit c1b736f Sep 17, 2024
23 checks passed
@emersion emersion deleted the emr/core-return-conflict-reqs branch September 17, 2024 15:25
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.

core, editoast: add requirements field to conflict list
5 participants