Skip to content

Add hotfix for broken occupancy bar.#167

Open
tynsh wants to merge 1 commit intoproductionfrom
hotfix/occupancy_bar
Open

Add hotfix for broken occupancy bar.#167
tynsh wants to merge 1 commit intoproductionfrom
hotfix/occupancy_bar

Conversation

@tynsh
Copy link
Copy Markdown
Contributor

@tynsh tynsh commented Nov 16, 2022

No description provided.

@tynsh
Copy link
Copy Markdown
Contributor Author

tynsh commented Nov 16, 2022

I don't know why the test runner fails. Since this is a hotfix, maybe you can run the tests on your machine and force a merge. Maybe you'll need to remove the setting on the project settings page requiring all checks to succeed even for administrators.

Copy link
Copy Markdown
Collaborator

@christian-heusel christian-heusel left a comment

Choose a reason for hiding this comment

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

I would propose a different fix 👍🏻 also we should add a test for this so it does not happen again 🤔
Let me know what you think, the only thing I would change is the default of occupancy level = 100% if there is no

Comment thread muesli/web/templates/Fragments/HTML/tutorial_occupancy.pt
Comment thread muesli/web/templates/Fragments/HTML/tutorial_occupancy.pt
@christian-heusel christian-heusel self-requested a review November 16, 2022 12:58
Copy link
Copy Markdown
Collaborator

@christian-heusel christian-heusel left a comment

Choose a reason for hiding this comment

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

Misunderstood the changes at first, LGTM! 😊
Test is still needed tho!

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