Skip to content

Conversation

@ckittl
Copy link
Member

@ckittl ckittl commented Jan 13, 2022

Resolves #128

@ckittl ckittl added the enhancement New feature or request label Jan 13, 2022
@ckittl ckittl added this to the Version 1.0 milestone Jan 13, 2022
@ckittl ckittl self-assigned this Jan 13, 2022
@sonarqubegithubprchecks

This comment has been minimized.

ckittl and others added 8 commits January 13, 2022 17:01
Co-authored-by: johanneshiry <johannes.hiry@tu-dortmund.de>
Co-authored-by: johanneshiry <johannes.hiry@tu-dortmund.de>
Co-authored-by: johanneshiry <johannes.hiry@tu-dortmund.de>
@sonarqubegithubprchecks

This comment has been minimized.

@codecov
Copy link

codecov bot commented Jan 14, 2022

Codecov Report

Merging #129 (f9d7b7f) into dev (6b1605b) will not change coverage.
The diff coverage is n/a.

❗ Current head f9d7b7f differs from pull request most recent head 551ffad. Consider uploading reports for the commit 551ffad to get more accurate results
Impacted file tree graph

@@          Coverage Diff          @@
##             dev    #129   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files         25      25           
  Lines       1512    1512           
  Branches     236     236           
=====================================
  Misses      1512    1512           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f65c45...551ffad. Read the comment docs.

@ckittl ckittl marked this pull request as ready for review January 14, 2022 07:50
@ckittl ckittl requested review from a team and johanneshiry January 14, 2022 07:53
@ckittl
Copy link
Member Author

ckittl commented Jan 14, 2022

@johanneshiry Please double check, if I took over your implementations correctly.

Copy link
Contributor

@t-ober t-ober left a comment

Choose a reason for hiding this comment

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

Some clarification questions and minor remarks.

Also: What about tests? Since I'm not too familiar with Akka they would make me feel better that this stuff works as expected. 🙄

idle(stopRunProcesses(guardianData, watch.runId, ctx))
case (ctx, unsupported) =>
ctx.log.error(s"Received unsupported message '$unsupported'.")
Behaviors.stopped
Copy link
Contributor

Choose a reason for hiding this comment

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

As of discussion: Check if a useful message is sent to the children.

Also the error message should include that we shut down the children and the guardian.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks to sealed trait Request, that case is unreachable, anyway.

Comment on lines 75 to 77
private def assignSubnetNumber(subGrid: SubGridContainer, subnetNumber: Int) =
subGrid
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is what we want 😂

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yes... If you forget to improve your own dummy implementation. ^^

Copy link
Member Author

Choose a reason for hiding this comment

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

Well... To not blow up this PR too much, I would like to hand this in as a separate PR (cf. #131).

@sonarqubegithubprchecks

This comment has been minimized.

@sonarqubegithubprchecks

This comment has been minimized.

@sonarqubegithubprchecks

This comment has been minimized.

@ckittl ckittl requested review from johanneshiry and t-ober January 24, 2022 14:47
@sonarqubegithubprchecks

This comment has been minimized.

@sonarqubegithubprchecks

This comment has been minimized.

@sonarqubegithubprchecks

This comment has been minimized.

@sonarqubegithubprchecks

This comment has been minimized.

@ckittl ckittl requested a review from johanneshiry January 27, 2022 11:03
@sonarqubegithubprchecks

This comment has been minimized.

Copy link
Member

@johanneshiry johanneshiry left a comment

Choose a reason for hiding this comment

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

Thanks for your work. Please consider my remarks and let me know if there is anything to discuss about. :)

@sonarqubegithubprchecks

This comment has been minimized.

@ckittl ckittl requested a review from johanneshiry January 27, 2022 16:48
@johanneshiry
Copy link
Member

!test

@sonarqubegithubprchecks
Copy link

Passed

Analysis Details

3 Issues

  • Bug0 Bugs
  • Vulnerability0 Vulnerabilities
  • Code Smell3 Code Smells

Coverage and Duplications

  • No coverage informationNo coverage information (0.00% Estimated after merge)
  • 3 percent duplication0.00% Duplicated Code (0.00% Estimated after merge)

Project ID: edu.ie3:OSMoGrid

View in SonarQube

Copy link
Member

@johanneshiry johanneshiry left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you! Wish you an amazing weekend! 🎖️

@t-ober t-ober merged commit 227815a into dev Jan 28, 2022
@t-ober t-ober deleted the ck/#128-parallelRuns branch January 28, 2022 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider parallel runs

5 participants