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

Add Data transformers for download #10

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

goncaloasimoes
Copy link
Collaborator

@goncaloasimoes goncaloasimoes commented Dec 28, 2020

Add transformers for specific formats. Here we add just for jed.
closes #2

@goncaloasimoes goncaloasimoes added the enhancement New feature or request label Dec 28, 2020
@goncaloasimoes goncaloasimoes self-assigned this Dec 28, 2020
Comment on lines +103 to +105
if ( class_exists( $transformer_class ) ) {
$transfomer = new $transformer_class();
return $transfomer->transform( $data, $config );
Copy link
Contributor

Choose a reason for hiding this comment

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

O $transformer_class que tens agora poderá ser a base, mas seria bom poder receber uma classe de fora para ser mais extensível. Futuro pull request ;)

O resto da lógica para remover complexidade e aumentar o que é testado.

Suggested change
if ( class_exists( $transformer_class ) ) {
$transfomer = new $transformer_class();
return $transfomer->transform( $data, $config );
if ( ! class_exists( $transformer_class ) ) {
return $data;
}
$transfomer = new $transformer_class();
if ( ! $transformer instanceof Transformer ) {
return $data;
}
return $transfomer->transform( $data, $config );

JSON_PRETTY_PRINT
);
private function apply_transformer( $data, Project $config ) {
//TODO: receive outside classes.
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't actually make sense in this way. What we'll have is a string with the class name for the transformer in the config file. So it'll not come as an argument, but will work mostly as is here.

It's more like this (just a rough example):

$transformer_class = $config->get_tranformer() ? $config->get_tranformer() : __NAMESPACE__ . '\\Transformers\\' . ucfirst( $config->get_format() );

@goncaloasimoes
Copy link
Collaborator Author

Reminder: careful with the change made in #36

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Separate concerns on Translations::download
3 participants