Skip to content

correct param, var and return annotation values #19325

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

Merged
merged 2 commits into from
Dec 5, 2018

Conversation

TomasVotruba
Copy link
Contributor

@TomasVotruba TomasVotruba commented Nov 22, 2018

This PR correct malformed annotations:

  • switched $type name
  • typos
  • $this to self that is recognized type by IDE
  • remove unused $value in @var and @return annotations

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Nov 22, 2018

CLA assistant check
All committers have signed the CLA.

@magento-engcom-team
Copy link
Contributor

Hi @TomasVotruba. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@sivaschenko
Copy link
Member

Code review passed, waiting for the builds to complete

@magento-engcom-team magento-engcom-team added this to the Release: 2.3.1 milestone Nov 22, 2018
@magento-engcom-team
Copy link
Contributor

Hi @sivaschenko, thank you for the review.
ENGCOM-3525 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

@TomasVotruba thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@TomasVotruba
Copy link
Contributor Author

TomasVotruba commented Nov 22, 2018

I've fixed one false positive. It's ready now

@sivaschenko Thanks for very fast review!

@TomasVotruba
Copy link
Contributor Author

Failing Travis is not related to this PR

Some Timeout - https://travis-ci.org/magento/magento2/jobs/458340630#L2557

@TomasVotruba
Copy link
Contributor Author

What needs to be done to merge this?

@sivaschenko
Copy link
Member

@TomasVotruba that pull request is currently in progress of delivery to mainline

@TomasVotruba
Copy link
Contributor Author

Thanks, I didn't know that

@magento-engcom-team
Copy link
Contributor

Hi @TomasVotruba. Thank you for your contribution.
We will aim to release these changes as part of 2.3.1.
Please check the release notes for final confirmation.

@TomasVotruba TomasVotruba deleted the doc branch December 5, 2018 12:51
@TomasVotruba
Copy link
Contributor Author

Glad to help!

Btw, it always takes ~10 days to merge such simple changes?

I want to send more similar PRs. But I'm afraid there will be many new code in the 10 days period and it will get stuck in rebase-resolve conflicts infinite loop.

@sivaschenko
Copy link
Member

@TomasVotruba I don't think it usually the case, there is a long internal queue of pull requests formed recently. Sorry for the delay.

@TomasVotruba
Copy link
Contributor Author

@sivaschenko I see. Is there some description of this flow?

Usually is 1-2 accepted reviews by maintainers + 3rd one marges.

@sivaschenko
Copy link
Member

@TomasVotruba after the pull request is approved on public GitHub it is moved to an internal repository for further QA and passing through internal CI test builds (it's only a small part of tests running on each pull request publicly).

@sidolov do we have any documentation on internal PR processing?

@sidolov
Copy link
Contributor

sidolov commented Dec 6, 2018

Hi @sivaschenko , we haven't such info right now

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.

5 participants