-
Notifications
You must be signed in to change notification settings - Fork 17
Prepare release notes for the v1.0.0-rc3 release #775
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
Conversation
The release notes were not cleared after the rc2 release, so we remove them now. Signed-off-by: Leandro Lucarella <[email protected]>
RELEASE_NOTES.md
Outdated
* The `propose_*` methods send power proposals to the `PowerManagingActor`, where it can be overridden by proposals from other actors. | ||
* They no longer have the `adjust_power` flag, because the `PowerManagingActor` will always adjust power to fit within the available bounds. | ||
* They no longer have a `include_broken_batteries` parameter. The feature has been removed. | ||
* `BatteryPool`: The `include_broken_batteries` argument was removed from the control methods. This option is no longer supported. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this belongs to - BatteryPool's control methods
which has been removed and not to microgrid package
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean because the BateryPool
object is defined in timeseries
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, I've just understood the changes done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ouch, please don't approve if you see any issues and auto-merge is enabled 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked at the commit and see this is not exposed via microgrid.battery_pool()
, so OK, will put it outside the microgrid
package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, this was very hard to track what was really changed and which parts were affected. @frequenz-floss/python-sdk-team when writing release notes we should think more from the user's perspective at the time to upgrade to write good notes, useful notes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to hopefully really reflect the user-visible changes and in a useful way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure what the confusion is, the flag was part of the battery pools control methods, and they've gone away: https://frequenz-floss.github.io/frequenz-sdk-python/v0.25/reference/frequenz/sdk/timeseries/battery_pool/#frequenz.sdk.timeseries.battery_pool.BatteryPool.charge
this was not about power distributing actor internals.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It affects the access via microgrid.battery_pool()
(which IMHO is the main thing the user cares about) and also some other public (although probably not used by users really) interfaces.
Signed-off-by: Leandro Lucarella <[email protected]>
f750d76
to
5554ed1
Compare
Remove notes from the rc2 release and update release notes for the v1.0.0-rc3 release.