8000 [Workflow] config doc by noniagriconomie · Pull Request #11580 · symfony/symfony-docs · GitHub
[go: up one dir, main page]
Skip to content

[Workflow] config doc #11580

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 1 commit into from
Closed

[Workflow] config doc #11580

wants to merge 1 commit into from

Conversation

noniagriconomie
Copy link
Contributor
@noniagriconomie noniagriconomie commented May 16, 2019 •
edited by OskarStark
Loading

Hi,

This draft PR follows this one
https://github.com/symfony/symfony-docs/pull/11492/files (config for 3.X version)
but for new version, mostly 4.3

I checked this PR also
symfony/symfony#30890, @lyrixx review appreciated please

Maybe to merge after merging #11578, and with a rebase ?

Draft for now

this one can be closed also:

@OskarStark
Copy link
Contributor

#11578 is merged, you can now rebase your branch 😃

@noniagriconomie noniagriconomie marked this pull request as ready for review May 17, 2019 15:53
@noniagriconomie noniagriconomie force-pushed the feature-workflow-config branch from 615058d to 9812a64 Compare May 20, 2019 07:38
OskarStark
OskarStark reviewed May 24, 2019
View reviewed changes
noniagriconomie
noniagriconomie commented May 27, 2019
View reviewed changes
workflow.rst
<framework:support>App\Entity\BlogPost</framework:support>

<framework:initial_marking>draft</framework:initial_marking>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@OskarStark really unsure of the pattern, I never use xml :/
Also i remove spaces like php and yml docbloc

Copy link
Contributor

Choose a reason for hiding this comment

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

I am unsure too about the xml config

cc @lyrixx

Copy link
Contributor

Choose a reason for hiding this comment

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

friendly ping @lyrixx can you please give us some feedback here?

@noniagriconomie
Copy link
Contributor Author

@OskarStark friendly ping, do i need to do other thing? thx

@noniagriconomie noniagriconomie deleted the feature-workflow-config branch June 25, 2019 15:14
@lyrixx
Copy link
Member
lyrixx commented Jun 25, 2019

Hello @noniagriconomie
Why did you close your PR?
8000

@noniagriconomie noniagriconomie restored the feature-workflow-config branch June 26, 2019 07:43
@noniagriconomie
Copy link
Contributor Author
noniagriconomie commented Jun 26, 2019 •
edited
Loading

@lyrixx fail branch removal, thank for noticing :)

I was cleaning my sf doc fork :/

@OskarStark OskarStark closed this Jun 26, 2019
@OskarStark
Copy link
Contributor

Argh wrong button

@OskarStark OskarStark reopened this Jun 26, 2019
@noniagriconomie noniagriconomie mentioned this pull request Jul 4, 2019
Merged
@OskarStark OskarStark added this to the 4.3 milestone Jul 30, 2019
@OskarStark OskarStark force-pushed the feature-workflow-config branch from d0b57a4 to c029eae Compare July 30, 2019 06:48
@OskarStark
Copy link
Contributor

@noniagriconomie can you please rebase your branch? Afterwards we are good to go 👍

@noniagriconomie
Copy link
Contributor Author

@OskarStark will do it asap, but i fear i will be afk soon ofr a short period of time :/
will do my best :)

@noniagriconomie
Copy link
Contributor Author

@OskarStark PR is ok for me

OskarStark
OskarStark reviewed Aug 12, 2019
View reviewed changes
reference/configuration/framework.rst
@@ -255,8 +255,9 @@ Configuration
* :ref:`name <reference-workflows-name>`

* `audit_trail`_
* `initial_place`_
Copy link
Contributor

Choose a reason for hiding this comment

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

@javiereguiluz is it ok to remove the deprecated one here? Or do we need to leave some info about the before option name?

@wouterj wouterj force-pushed the feature-workflow-config branch from f9c506a to 3583a4c Compare August 12, 2019 12:22
@wouterj wouterj mentioned this pull request Aug 12, 2019
Merged
@wouterj
Copy link
Member
wouterj commented Aug 12, 2019

Soo as you can see I completely destroyed your branch here. Fortunately, GitHub keeps all commit refs in your fork, so I was able to correctly rescue all your amazing changes and created a new PR: #12146

I've also checked the XML code and it looks perfect. One minor thing: While both underscore and dash are valid in Symfony configuration, using the dash is considered better XML, so I've directly updated the one occurence of underscore.

Also, it appeared that the dumping article was already rewritten during the opening of this PR and now. So I've removed that from the changes.

Thanks a lot and sorry for our slow responses on this PR.

@wouterj wouterj closed this Aug 12, 2019
wouterj added a commit that referenced this pull request Aug 12, 2019
@wouterj
feature #12146 [4.3] Update workflow config (noniagriconomie, wouterj)
aea2b64 
This PR was merged into the 4.3 branch.

Discussion
----------

[4.3] Update workflow config

Rescued some really bad git stuff from me while merging: #11580

Original description:

> Hi,
>
> This draft PR follows this one
> https://github.com/symfony/symfony-docs/pull/11492/files (config for 3.X version)
> but for new version, mostly 4.3
>
> I checked this PR also
> symfony/symfony#30890, @lyrixx review appreciated please
>
> Maybe to merge after merging #11578, and with a rebase ?
>
> --------------------
>
> Draft for now
>
> - Closes #11229
>
> this one can be closed also:
>
> - Closes #9475
> - Closes #10197

Commits
-------

fdc221b Minor XML improvement
a640d14 [Workflow] config doc
7bb2041 Fix indent
f65f1bc Review
cf3a32c Doc config metadata
34e7cc5 [Workflow] config doc
@noniagriconomie
Copy link
Contributor Author

@wouterj thank you

@noniagriconomie noniagriconomie deleted the feature-workflow-config branch August 13, 2019 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OskarStark OskarStark OskarStark left review comments

@lyrixx lyrixx lyrixx approved these changes

@xabbuh xabbuh Awaiting requested review from xabbuh xabbuh is a code owner

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

Successfully merging this pull request may close these issues.
5 participants
@noniagriconomie @OskarStark @lyrixx @wouterj @carsonbot
0