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

Dynaflow refactor #130

Merged
merged 17 commits into from
Nov 25, 2022
Merged

Dynaflow refactor #130

merged 17 commits into from
Nov 25, 2022

Conversation

dimbdr
Copy link
Contributor

@dimbdr dimbdr commented Nov 18, 2022

Please check if the PR fulfills these requirements (please use '[x]' to check the checkboxes, or submit the PR and then click the checkboxes)

  • The commit message follows our guidelines

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Mainly changing the IIDM version.

BAUDRIER Dimitri added 3 commits November 18, 2022 11:13
- iidm version
- version of Dynaflow API
- Dynaflow name

Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
BAUDRIER Dimitri added 3 commits November 18, 2022 11:22
Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
Co-authored-by: Florian Dupuy <66690739+flo-dup@users.noreply.github.com>
Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
BAUDRIER Dimitri added 2 commits November 24, 2022 14:28
Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
@dimbdr dimbdr requested a review from flo-dup November 24, 2022 13:33
BAUDRIER Dimitri added 3 commits November 25, 2022 15:43
Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
@@ -74,6 +76,11 @@ public void checkExecutionCommand() {
assertEquals(expectedExecutionCommand, executionCommand);
}

@Test
public void iidmCurrentVersionUsed() {
assertEquals(IidmXmlVersion.V_1_4.toString("."), IIDM_VERSION);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can remove that test which is not that useful, plus it produces a code smell.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

return this.toString(DEFAULT_DELIMITER);
}

public static DynaFlowVersion of(String version) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You could also return an Optional<DynaFlowVersion> in both DynaFlowVersion::of, it would make the code a bit clearer when checking the version don't you think? with a .map(v -> v.compareTo(version) >= 0).orElse(false) instead of catching an exception

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh nice, it's quite better indeed!
Done!

BAUDRIER Dimitri added 2 commits November 25, 2022 17:15
Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
Signed-off-by: BAUDRIER Dimitri <dimitri.baudrier@rte-france.com>
@dimbdr dimbdr requested a review from flo-dup November 25, 2022 16:31
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@flo-dup flo-dup merged commit ddae662 into main Nov 25, 2022
@flo-dup flo-dup deleted the dynaflow-refactor branch November 25, 2022 16:48
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.

4 participants