Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Add getCacheId() and clearCache() method to Translator #93

Closed
wants to merge 3 commits into from
Closed

Add getCacheId() and clearCache() method to Translator #93

wants to merge 3 commits into from

Conversation

MatthiasKuehneEllerhold
Copy link
Contributor

See #63 for discussion.

Copy link
Member

@malukenho malukenho left a comment

Choose a reason for hiding this comment

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

I’m missing tests for the introduced api

* Get the cache identifier for a specific textDomain and locale.
*
* @param string $textDomain
* @param string $locale
Copy link
Member

Choose a reason for hiding this comment

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

Too much space

* Clears the cache for a specific textDomain and locale.
*
* @param $textDomain
* @param $locale
Copy link
Member

Choose a reason for hiding this comment

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

Parameter types?

@MatthiasKuehneEllerhold
Copy link
Contributor Author

MatthiasKuehneEllerhold commented Apr 30, 2018

Fixed the comments and added tests.

Is there any consensus in the ZF code style about using static methods via $this ? (See Unit-Tests $this->assertXY...

@weierophinney
Copy link
Member

Is there any consensus in the ZF code style about using static methods via $this ?

For unit tests, that's the style we've consistently done since, well, we began. There's an open discussion in our forums covering exactly this, though with no consensus as of yet.

weierophinney added a commit that referenced this pull request May 16, 2018
Add getCacheId() and clearCache() method to Translator
weierophinney added a commit that referenced this pull request May 16, 2018
weierophinney added a commit that referenced this pull request May 16, 2018
@weierophinney
Copy link
Member

Thanks, @MatthiasKuehneEllerhold. Merged to develop for release with 2.9.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants