Skip to content

[Workflow] Guard documentation #11421

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

[Workflow] Guard documentation #11421

wants to merge 2 commits into from

Conversation

noniagriconomie
Copy link
Contributor

Hi,

Here is a draft PR for this issue #11214

@pbowyer @xabbuh @javiereguiluz Am I in the right direction?

Thank you,

@noniagriconomie noniagriconomie force-pushed the feature-workflow-guard branch from b9e1981 to 3c9ed1d Compare April 12, 2019 14:59
@noniagriconomie noniagriconomie force-pushed the feature-workflow-guard branch from 3c9ed1d to d86dc6d Compare April 12, 2019 15:04
javiereguiluz
javiereguiluz reviewed Apr 12, 2019
View reviewed changes
Copy link
Member

@javiereguiluz javiereguiluz left a comment

Choose a reason for hiding this comment

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

Thanks Antoine! It looks nice so far, but I'm not expert in Workflow, so we'll need more reviews.

pbowyer
pbowyer reviewed Apr 12, 2019
View reviewed changes
Copy link
Contributor

@pbowyer pbowyer left a comment

Choose a reason for hiding this comment

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

Thanks for writing this! It is a great beginning; my changes are to do with the English rather than the technical side of using Workflow Guards. More rewording will be needed, but let's get the content approved first.

@noniagriconomie noniagriconomie marked this pull request as ready for review April 15, 2019 05:20
@noniagriconomie
Copy link
Contributor Author

@pbowyer @javiereguiluz review addressed

For the "technical side content" you mentioned, do you see any other workflow guard feature needed to document here?

@pbowyer
Copy link
Contributor

pbowyer commented Apr 15, 2019

@noniagriconomie I'm learning Workflow as I go, so I'm not the best person to give a technical review. Pinging @lyrixx and @HeahDude who know a lot!

@lyrixx
Copy link
Member

lyrixx commented Apr 15, 2019

Hello,

Thanks for your PR.
Guard are supported by the workflow from day 1. So could tou rebase your branch on the lowest supported branch: 3.4?

@noniagriconomie noniagriconomie changed the base branch from master to 3.4 April 15, 2019 13:07
@noniagriconomie noniagriconomie changed the base branch from 3.4 to master April 15, 2019 13:07
@noniagriconomie
Copy link
Contributor Author

noniagriconomie commented Apr 15, 2019 •
edited
Loading

yes i will do it
closed in favor of #11433

@noniagriconomie noniagriconomie mentioned this pull request Apr 15, 2019
Closed
@noniagriconomie noniagriconomie deleted the feature-workflow-guard branch April 15, 2019 13:13
javiereguiluz added a commit that referenced this pull request Apr 16, 2019
@javiereguiluz
minor #11433 [Workflow] Guard documentation (noniagriconomie)
4fbca85 
This PR was squashed before being merged into the 3.4 branch (closes #11433).

Discussion
----------

[Workflow] Guard documentation

Hi,

Here is a PR for this issue #11214

See #11421

Commits
-------

07ada7f [Workflow] Guard documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@javiereguiluz javiereguiluz javiereguiluz left review comments

@pbowyer pbowyer pbowyer left review comments

Assignees
No one assigned
Labels
Status: Needs Review Workflow
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@noniagriconomie @pbowyer @lyrixx @javiereguiluz @carsonbot