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

Include the installed state in the template for toggling pause/stop #795

Closed
wants to merge 2 commits into from

Conversation

kinow
Copy link
Member

@kinow kinow commented Oct 6, 2021

These changes close #748

Requirements check-list

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Appropriate tests are included (unit and/or functional).
  • Appropriate change log entry included.
  • No documentation update required.

@kinow kinow added this to the 0.6.0 milestone Oct 6, 2021
@kinow kinow self-assigned this Oct 6, 2021
@kinow
Copy link
Member Author

kinow commented Oct 6, 2021

Simply handling a new initial state that we have: installed. Here's the manual tests performed.

A installed, but never run, workflow:

image

A stopped workflow:

image

A running workflow:

image

A paused workflow (reloaded browser to confirm it's not only a local state):

image

A running (restart?) workflow:

image

Stopped again workflow:

image

@kinow kinow marked this pull request as ready for review October 6, 2021 23:36
@oliver-sanders
Copy link
Member

We should really get rid of the installed state before release, putting up some PRs to address this...

@hjoliver
Copy link
Member

(Presumably #805 makes this redundant?)

@hjoliver
Copy link
Member

Sorry @kinow it looks like this one won't be needed now - OK?

@kinow
Copy link
Member Author

kinow commented Oct 12, 2021

Sorry @kinow it looks like this one won't be needed now - OK?

Yup! Not a problem, important thing is it was fixed :) closing now, thanks!

@kinow kinow closed this Oct 12, 2021
@kinow kinow deleted the include-installed-state branch October 12, 2021 21:41
@hjoliver
Copy link
Member

Oops, hang on, sorry! #748 is about greying out the play icons, not just the about-to-be-removed "installed" state. So maybe this PR could be adapted once installed is removed?

@hjoliver
Copy link
Member

Luckily branch deletion is reversible 😁

@kinow kinow restored the include-installed-state branch October 12, 2021 21:45
@kinow kinow reopened this Oct 12, 2021
@kinow
Copy link
Member Author

kinow commented Oct 12, 2021

Branch restored! 😌

@hjoliver
Copy link
Member

Damn it, premature branch restoration!

I just ran the #805 branches and it looks like the icons are correctly greyed there for not-yet-run workflows.

@oliver-sanders
Copy link
Member

I think #805 has now superseded this the lazy way (by deleting the extra state!).

@hjoliver
Copy link
Member

OK you can delete your branch again @kinow 😁

@hjoliver hjoliver closed this Oct 15, 2021
@kinow kinow deleted the include-installed-state branch October 15, 2021 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong status in Toolbar
3 participants