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

[Docs] Fixing php-errors in the documentation #11619

Merged
merged 3 commits into from
Jul 9, 2020

Conversation

mamazu
Copy link
Member

@mamazu mamazu commented Jul 5, 2020

Q A
Branch? master
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Related tickets -none-
License MIT

I am currently developing a documentation validator and added an rst parser to it. Sylius was the trail run project and this is what I got out of it (fixes are done manually).

One thing I am struggling with a little is the ... situation. Having ... in a documentation is not very helpful, because to me it is saying: "You know what to fill in here". But then why write the documentation in the first place. (eg. SyliusOrderBundle/processors.rst) So I am not sure about that.

@mamazu mamazu requested a review from a team as a code owner July 5, 2020 00:27
@probot-autolabeler probot-autolabeler bot added the Documentation Documentation related issues and PRs - requests, fixes, proposals. label Jul 5, 2020
@@ -31,7 +31,6 @@ Summary
interface: Sylius\Component\Promotion\Model\PromotionRuleInterface
controller: Sylius\Bundle\ResourceBundle\Controller\ResourceController
repository: ~
factory: Sylius\Component\Resource\Factory\Factory
Copy link
Member Author

Choose a reason for hiding this comment

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

This definition is there twice and yaml does not allow that.

@mamazu mamazu force-pushed the documentation_fixes branch from 253dcd7 to bae25d9 Compare July 5, 2020 12:22
Copy link
Member

@lchrusciel lchrusciel left a comment

Choose a reason for hiding this comment

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

Thanks a lot for these fixes. Let's change the order processor and it is good to be merged ;)

@mamazu
Copy link
Member Author

mamazu commented Jul 6, 2020

Agreed. You usually want the composite one so I guess this is the best way to document it.

@lchrusciel
Copy link
Member

@mamazu can you rebase to the master, please?

mamazu added 2 commits July 7, 2020 17:25
In the documentation you usually want the composite processor so let's
document it that way.
@mamazu mamazu force-pushed the documentation_fixes branch from d004766 to 98422f1 Compare July 7, 2020 15:25
@mamazu
Copy link
Member Author

mamazu commented Jul 7, 2020

Should be done now. I have also fixed on error that emerged from it. (Apparently sphinx checks if JSON data is parsable but not PHP.)

Comment on lines 40 to 41
)
{
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
)
{
) {

@lchrusciel lchrusciel merged commit 08e3351 into Sylius:master Jul 9, 2020
@lchrusciel
Copy link
Member

Thanks, @mamazu! 🎉

@mamazu mamazu deleted the documentation_fixes branch October 16, 2020 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Documentation related issues and PRs - requests, fixes, proposals.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants