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

Freeze Queue #224

Merged
merged 4 commits into from
Feb 22, 2020
Merged

Conversation

ThrowsException
Copy link
Collaborator

#214

Makes the program not selectable from the participant info form if the program has been marked as frozen from the queue

Copy link
Collaborator

@MikeyManoguerra MikeyManoguerra left a comment

Choose a reason for hiding this comment

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

It looks good to me as long as it is functioning.
i made one small comment about inline styles, but otherwise, get the branch up to date and ill approve it

@ThrowsException
Copy link
Collaborator Author

@MikeyManoguerra thanks for the review. I revamped this a little bit. Instead of just blindly flipping the state when its clicked the button will wait for a valid server response. I also went a fixed the AllQueue.test.js file since it was failing due to some other PR's. Also added a test for the button. Probably want to give this a once over again.

@MikeyManoguerra
Copy link
Collaborator

Looks good!
Its ok to merge, i believe convention still is once you get approved you merge your own changes.

Some things outside the scope of this ticket we'll need to think about:

  • the difference between "frozen" and "closed".
  • A daily cycle : does "is_ frozen" reset to false every day? This can fold into some sort of daily reset routine, That should be discussed in full because there are definitely other undecided lifecycle type things to consider

@ThrowsException
Copy link
Collaborator Author

@MikeyManoguerra Yea I noticed the is closed. if its decided later to use that instead it should be easy to switch over. ill get the branch updated and merged.

@ThrowsException
Copy link
Collaborator Author

ThrowsException commented Feb 20, 2020

I guess by rebasing it invalidates the review. Sorry mind approving again and ill get this merged. @MikeyManoguerra

@MikeyManoguerra
Copy link
Collaborator

@ThrowsException they're moving too fast! message me on slack and ill see this quicker

ThrowsException and others added 4 commits February 21, 2020 19:13
Makes the program not selectable from the participant info form if the program has been marked as frozen from the queue
Copy link
Collaborator

@MikeyManoguerra MikeyManoguerra 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!

@ThrowsException ThrowsException merged commit 1b2620a into CodeForPhilly:master Feb 22, 2020
@ThrowsException ThrowsException deleted the freeze-queue branch February 22, 2020 00:18
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