Skip to content
This repository has been archived by the owner on Nov 7, 2020. It is now read-only.

B1 refactor #55

Merged
merged 2 commits into from
Mar 29, 2020
Merged

B1 refactor #55

merged 2 commits into from
Mar 29, 2020

Conversation

jamesclawley
Copy link
Contributor

Adds a new class for presentation slides to reduce load on the presentation class

@sleepy-evelyn
Copy link
Contributor

sleepy-evelyn commented Mar 29, 2020

So there are some issues with the recent mealplanner merge that means I can't really test it.
about to post these in github issues.

For this reason it makes sense to wait until the mealplanner is fixed before doing this.
If someone can immediately re-open the mealplanner branch or revert the mealplanner merge that would be handy. Thanks.

@nathanbillis
Copy link
Contributor

So there are some issues with the recent mealplanner merge that means I can't really test it.
For this reason it makes sense to wait until the mealplanner is fixed before doing this.
If someone can immediately re-open the mealplanner branch that would be handy. Thanks.

Make a new branch and work on the bug fixes, what are the issues with meal planner?

@nathanbillis
Copy link
Contributor

nathanbillis commented Mar 29, 2020

For this reason it makes sense to wait until the mealplanner is fixed before doing this.

What would make sense would be to merge this, then open a new branch from the head of this and work on the fixes for the meal planner.

@sleepy-evelyn
Copy link
Contributor

sleepy-evelyn commented Mar 29, 2020

True. That would work. I'd say with this branch re-opened though it might make sense to write some tests for it whilst we can to make sure we don't run into issues further down the line. This will involve me re-opening my own branch for audio and timers as well but it's probably worth it. Obviously takes more time though.

In the end though this isn't really my call so if you think it's excessive or just need to merge anyway to have an updates presentation class go for it.

p.s. this is based off the fact i've just discovered a 'testBaconComment' thing has been added which essentially allows us to navigate to specific recipes to test. No idea who wrote it though.

@sleepy-evelyn
Copy link
Contributor

Also kudos to James to refactoring tho and comments. A lot easier to understand now.

@nathanbillis
Copy link
Contributor

nathanbillis commented Mar 29, 2020

I'm going to merge this, then we can create new branches for the audio & timers and for any other issues we find later on.

We shouldn't be reopening old branches once they're merged into the master because it will cause alot of issues later on. If there's something forgotten or needs changing later on then a new branch should be made with a new PR when its finished.

@nathanbillis nathanbillis mentioned this pull request Mar 29, 2020
3 tasks
@nathanbillis nathanbillis added this to the 2nd Iteration milestone Mar 29, 2020
@nathanbillis
Copy link
Contributor

Merging and closing this branch, any new issues found after basic troubleshooting should be opened and new branches made,

@nathanbillis nathanbillis merged commit 7a6c6cd into master Mar 29, 2020
@nathanbillis nathanbillis deleted the B1_Refactor branch March 29, 2020 16:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Patch bug fixes or improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants