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

GSYE-767: Enable passing profiles to OrderUpdaterParameters #1795

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

hannesdiedrich
Copy link
Member

Reason for the proposed changes

Please describe what we want to achieve and why.

Proposed changes

INTEGRATION_TESTS_BRANCH=master
GSY_FRAMEWORK_BRANCH=master

Copy link
Contributor

@BigTava BigTava left a comment

Choose a reason for hiding this comment

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

LG, but left 1 comment

return self.update_interval

@staticmethod
def _get_default_value_initial_rate(_time_slot: DateTime):
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm, it feels we can simplify here no and just return 0 instead of calling this function? Or the idea is to have a default value for every time_slot?

Copy link
Member Author

Choose a reason for hiding this comment

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

Please have a look at the HeatPumpOrderUpdaterParameters that inherits from the OrderUpdaterParameters. This is to inject a different default value to every implementation of OrderUpdaterParameters. Here we do not need a specific default value. Does it make sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see we are overriding the function. Looks fine then

@hannesdiedrich hannesdiedrich merged commit 39c436f into master Sep 19, 2024
2 checks passed
@hannesdiedrich hannesdiedrich deleted the feature/GSYE-767 branch September 19, 2024 14:01
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.

2 participants