Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Documented the workflow metadata [redux]#11209

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@pbowyer
Copy link
Contributor

With@javiereguiluz's permission I have taken#9476 and incorporated the comments and code examples into it.

This is the start of a push to improve the Workflow documentation. The focus now is on expanding the documentation by merging pull requests; organizing it will come later.

javiereguiluz reacted with thumbs up emoji
@HeahDudeHeahDude added this to the4.2 milestoneMar 23, 2019
Copy link
Contributor

@HeahDudeHeahDude left a comment

Choose a reason for hiding this comment

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

This a great contribution, thanks a lot!

pbowyer added a commit to pbowyer/symfony-docs that referenced this pull requestMar 23, 2019
pbowyer added a commit to pbowyer/symfony-docs that referenced this pull requestMar 23, 2019
Whensymfony#9465 is finished, link to details on Transition Blockers.
Copy link
Contributor

@HeahDudeHeahDude left a comment

Choose a reason for hiding this comment

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

I've left some minor comments, but that looks great. Thank you again for your work on this.

Note that we can do some minor changes when merging if you don't want to bother with specific formatting.

@pbowyer
Copy link
ContributorAuthor

@HeahDude re "specific formatting" I've noticed the file uses a mix ofarray( ... ) and[ ... ]. Which should be used?

When I took Javier's commits the branch ended up being created againstsymfony-docs/master. Can you make it be against the right branch when you merge?

@HeahDude
Copy link
Contributor

I've noticed the file uses a mix of array( ... ) and [ ... ]. Which should be used?

Now we use the new short syntax, since the lowest supported version is 3.4 having PHP 5.5 as minimum.

When I took Javier's commits the branch ended up being created against symfony-docs/master. Can you make it be against the right branch when you merge?

4.1 was master back then, now we need to merge into 4.2 but we can do it on merge thanks to the CLI tool we use to maintain the docs.
Thanks!

@pbowyer
Copy link
ContributorAuthor

Re the code style comment above which is outstanding: Is there a linter for code examples? An automated formatter?

I imagine we stick to Symfony's code style, but as Phpstorm's Symfony2 preset seems to differ in small ways from the current Symfony format, it would be good to have a definitive formatter to use.

Copy link
Contributor

@OskarStarkOskarStark left a comment

Choose a reason for hiding this comment

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

I left a few comments

Copy link
Member

@lyrixxlyrixx left a comment

Choose a reason for hiding this comment

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

Except one little comment big 👍

Thanks you a lot for your work on this PR

@pbowyer
Copy link
ContributorAuthor

@OskarStark Your code review is showing as "Changes requested" - are you satisfied with the changes that were applied?

@lyrixx
Copy link
Member

lyrixx commentedApr 7, 2019•
edited
Loading

@pbowyer This PR conflicts. Could you try to rebase it and fix the conflict? Thanks

@pbowyer
Copy link
ContributorAuthor

@lyrixx Of course. 4.2 is the correct one to rebase onto, I believe?

@lyrixx
Copy link
Member

Exactly 👍 You need to rebase on top of 4.2 and update the target branch on github
Thanks again

@pbowyerpbowyerforce-pushed thepull/9476-workflow-metadata branch frome580f3c to6080aa8CompareApril 7, 2019 09:40
@pbowyerpbowyer changed the base branch frommaster to4.2April 7, 2019 09:41
@pbowyer
Copy link
ContributorAuthor

@lyrixx There we go 😁

@pbowyer
Copy link
ContributorAuthor

There may be one newline missing:

As configured, the following property is used by the marking store::.. code-block:: php

I think there should be a blank line between the two?

Copy link
Contributor

@OskarStarkOskarStark left a comment

Choose a reason for hiding this comment

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

Thank you for your patience ✌🏻

return;
}

$explanation = $event->getMetadata('explanation', $event->getTransition());
Copy link
Contributor

@noniagriconomienoniagriconomieApr 9, 2019•
edited
Loading

Choose a reason for hiding this comment

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

@pbowyer I think you should add anexplanation entry in the metadata of onepublish transition so that a newcomer can directly spot the usage and the link between transition, place, etc

@noniagriconomie
Copy link
Contributor

Ref to#9475

@pbowyer
Copy link
ContributorAuthor

Can we merge this please? There are more PRs for Workflow coming to fruition, and if they all get approved/merged at the same time, there will be a number of conflicts...

@javiereguiluzjaviereguiluz merged commit7f3a0fd intosymfony:4.2Apr 17, 2019
javiereguiluz added a commit that referenced this pull requestApr 17, 2019
… pbowyer, OskarStark)This PR was merged into the 4.2 branch.Discussion----------Documented the workflow metadata [redux]With@javiereguiluz's permission I have taken#9476 and incorporated the comments and code examples into it.This is the start of a push to improve the Workflow documentation. The focus now is on expanding the documentation by merging pull requests; organizing it will come later.Commits-------7f3a0fd Oskar's feedbacka4c23c1 Add blank line between code block and sentence above. Code block was not rendering.6080aa8 Remove the word 'simple'3765ddb Change code formatting of PHP snippet perhttps://github.com/symfony/symfony-docs/pull/11209/files/d57fa38d903175d58d9cfbf63f20e7ced8d2fd01#r26839165555c9199 Indent PHP block by an additional 4 spaces225c2fe Apply suggestions from code reviewf716e81 Incorporate@OskarStark's feedback94d17de Simplify the English and turn it into a tip40fbaf3 Update arrays to use short syntaxea64992 Incorporate further excellent feedback from@HeahDudeaf71c14 Add an introduction as suggested in#11209 (comment)ca66356 Document how to see all configuration options (see#11209 (comment))c9ef262 Incorporate@OskarStark's feedbackc595c47 First set of tidy-ups for@HeahDude's feedback.663639b Incorporate the code examples in#9476 (comment) into the documentation17eae8c Add missing metadata key, fixing comment by@noniagriconomie on#9476d68cc99 Documented the workflow metadata
@javiereguiluz
Copy link
Member

Thanks Peter! This has been merged ... and I'll rebase the Workflow doc reorganization (#11437) with your changes.

@OskarStark
Copy link
Contributor

Great work Peter, thank you 🙏

alexislefebvre reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@javiereguiluzjaviereguiluzjaviereguiluz left review comments

@OskarStarkOskarStarkOskarStark approved these changes

+4 more reviewers

@HeahDudeHeahDudeHeahDude left review comments

@noniagriconomienoniagriconomienoniagriconomie left review comments

@lyrixxlyrixxlyrixx approved these changes

@maxheliasmaxheliasmaxhelias approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.2

Development

Successfully merging this pull request may close these issues.

8 participants

@pbowyer@HeahDude@lyrixx@noniagriconomie@javiereguiluz@OskarStark@maxhelias@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp