Skip to content

Conversation

@ihor-sviziev
Copy link
Contributor

Description

In #13770 was introduced new property with protected visibility, should be private.
This commit wasn't included into any release, so we still can fix this issue.

Fixed Issues (if relevant)

N/A

Manual testing scenarios

N/A

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

This property was introduced in b1a5bc1 with protected visibility
@r-martins
Copy link
Contributor

r-martins commented Mar 7, 2018

@ihor-sviziev sorry for not seeing your previous comment on #13770.
I'm just wondering why $allowedLocales should be kept protected while $allowedCurrencies, private.
Thanks mate.

Update: Both variables has protected visibility, and has public methods to get its information. I also checked that there's no other usage outside the class for both properties. Can you confirm that? Perhaps both might be private...

@ihor-sviziev ihor-sviziev deleted the setup-lists-make-allowed-currencies-private branch March 7, 2018 05:37
@ihor-sviziev
Copy link
Contributor Author

Hi @r-martins,
Unfortunately such change will be not backward compatible as it was already included as protected in one of releases.

@r-martins
Copy link
Contributor

r-martins commented Mar 8, 2018 via email

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.

4 participants