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

added docs to Versions::VERSION #81

Merged
merged 3 commits into from
Mar 25, 2019
Merged

added docs to Versions::VERSION #81

merged 3 commits into from
Mar 25, 2019

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Mar 25, 2019

the constant is public for technical reasons, therefore should be documented as @internal.

Added a hint for the public api to use

the constant is public for technical reasons, therefore should be documented as `@internal`.

Added a hint for the public api to use
@staabm
Copy link
Contributor Author

staabm commented Mar 25, 2019

lead by my IDEs autocomplete feature I accidentally read from this public array, instead of using the proper API. this mistake on my end resulted in PackageVersions not using the proper Fallback for the --no-scripts case.

@@ -48,6 +48,12 @@ final class Installer implements PluginInterface, EventSubscriberInterface
%s
{
public const ROOT_PACKAGE_NAME = '%s';
/**
* Array of all available composer packages.
Copy link
Owner

Choose a reason for hiding this comment

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

Can you also add the type of this array? It is array<string, string>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

@staabm
Copy link
Contributor Author

staabm commented Mar 25, 2019

should I adjust test expectations or is this something you can do easily?

@Ocramius
Copy link
Owner

Yeah, test expectations also need adjusting

@Ocramius Ocramius self-assigned this Mar 25, 2019
@Ocramius Ocramius added this to the 1.5.0 milestone Mar 25, 2019
@Ocramius Ocramius merged commit 8b9fddd into Ocramius:master Mar 25, 2019
@Ocramius
Copy link
Owner

Thanks @staabm!

@staabm staabm deleted the patch-1 branch March 25, 2019 12:24
* Array of all available composer packages.
* Dont read this array from your calling code, but use the \PackageVersions\Versions::getVersion() method instead.
*
* @var array<string, string>
Copy link
Contributor

Choose a reason for hiding this comment

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

@var is invalid for constants.

Copy link
Owner

Choose a reason for hiding this comment

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

I'd prefer keeping it anyway

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.

None yet

3 participants