Skip to content

Adapt default resolution of weather source #79

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Apr 7, 2022
Merged

Adapt default resolution of weather source #79

merged 13 commits into from
Apr 7, 2022

Conversation

ckittl
Copy link
Member

@ckittl ckittl commented Jan 11, 2022 •
edited
Loading

Resolves #78
Resolves #173

@ckittl ckittl added the enhancement New feature or request label Jan 11, 2022
@ckittl ckittl requested a review from a team January 11, 2022 09:21
sebastian-peter
sebastian-peter requested changes Feb 9, 2022
View reviewed changes
Copy link
Member

@sebastian-peter sebastian-peter left a comment

Choose a reason for hiding this comment

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

I think the change regarding EMPTY_WEIGHT_SUM is not thought through. Other than that all good

t-ober
t-ober requested changes Mar 2, 2022
View reviewed changes
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.

Also scaladocs for the WeightSum class would be great so it is more clear what it represents how it is supposed to work

@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@codecov
Copy link

codecov bot commented Mar 14, 2022 •
edited
Loading

Codecov Report

Merging #79 (47530b3) into dev (ad824fc) will decrease coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##              dev      #79      +/-   ##
==========================================
- Coverage   79.52%   79.48%   -0.05%     
==========================================
  Files         155      156       +1     
  Lines        5749     5757       +8     
  Branches       79       79              
==========================================
+ Hits         4572     4576       +4     
- Misses       1177     1181       +4
Impacted Files Coverage Δ
...edu/ie3/simona/service/weather/WeatherSource.scala 76.57% <0.00%> (ø)
...rc/main/scala/edu/ie3/util/scala/DoubleUtils.scala 50.00% <0.00%> (ø)
.../simona/service/weather/WeatherSourceWrapper.scala 43.90% <0.00%> (+0.48%) ⬆️

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 ad824fc...47530b3. Read the comment docs.

@ckittl ckittl assigned ckittl and unassigned johanneshiry Mar 15, 2022
sebastian-peter
sebastian-peter requested changes Mar 16, 2022
View reviewed changes
Copy link
Member

@sebastian-peter sebastian-peter 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 overall, just minor concerns

t-ober
t-ober requested changes Mar 16, 2022
View reviewed changes
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
t-ober
t-ober previously approved these changes Mar 31, 2022
View reviewed changes
@sebastian-peter sebastian-peter self-assigned this Apr 5, 2022
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
4 similar comments
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@sonarqubegithubprchecks
Copy link

Passed

Analysis Details

1 Issue

  • Bug0 Bugs
  • Vulnerability0 Vulnerabilities
  • Code Smell1 Code Smell

Coverage and Duplications

  • 60 percent coverage77.27% Coverage (81.70% Estimated after merge)
  • 3 percent duplication0.00% Duplicated Code (0.00% Estimated after merge)

Project ID: edu.ie3:simona

View in SonarQube

@sebastian-peter sebastian-peter requested review from t-ober and danielfeismann and removed request for sebastian-peter April 6, 2022 12:04
danielfeismann
danielfeismann approved these changes Apr 7, 2022
View reviewed changes
Copy link
Member

@danielfeismann danielfeismann left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the work! :)

@danielfeismann danielfeismann merged commit f10db48 into dev Apr 7, 2022
@danielfeismann danielfeismann deleted the ck/#78-weatherSourceDefaultResolution branch April 7, 2022 07:23
@sebastian-peter sebastian-peter added this to the Version 3.0 milestone Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebastian-peter sebastian-peter sebastian-peter approved these changes

@danielfeismann danielfeismann danielfeismann approved these changes

@t-ober t-ober Awaiting requested review from t-ober

Assignees

@sebastian-peter sebastian-peter

@ckittl ckittl

Labels
enhancement New feature or request
Projects
None yet
Milestone
Version 3.0
Development

Successfully merging this pull request may close these issues.

Improve weighting of weather from different coordinates Fix weather source default resolution
5 participants
@ckittl @sebastian-peter @t-ober @danielfeismann @johanneshiry