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

Fixes #7156: Modify generic methods to define and use the new class_prefix #226

Conversation

peckpeck
Copy link
Member

@peckpeck peckpeck commented Sep 4, 2015

methods:
"placeholder" usebundle => file_copy_from_local_source_recursion("${source}", "${destination}", "0");
"copy without recursion" usebundle => file_copy_from_local_source_recursion("${source}", "${destination}", "0");
"new result classes" usebundle => _classes_copy("${class_prefix}", "${new_class_prefix}");
Copy link
Member

Choose a reason for hiding this comment

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

i'm not sure of the result here.
If you call twice file_copy_from_local_source_recursion, with same destination, you won't be able to differenciate the results (the class_prefix will be the same)

Copy link
Member Author

Choose a reason for hiding this comment

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

The second time you call the file_copy_from_local_source method, you will have a different promiser, which will give a different stack from here, hence a different new_class_prefix.
We will still have a common class_prefix, but we we cannot get rid of it until we are sure that the promiser stack feature is supported.

Copy link
Member

Choose a reason for hiding this comment

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

_class_copy cpy class_prefix (old value) into new_class_prefix (new value)
So the new promiser will be overriden by the old one (no way to distringuish if it is the previous call or this call that defined the _error or the _success)

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yes, then we'll have to define the classes at the source, ie define both in a new body classes

@peckpeck
Copy link
Member Author

peckpeck commented Sep 7, 2015

PR updated

@peckpeck peckpeck force-pushed the arch_7156/modify_generic_methods_to_define_and_use_the_new_class_prefix branch from f1fd4b6 to 57b6d24 Compare September 7, 2015 12:48
@peckpeck
Copy link
Member Author

peckpeck commented Sep 8, 2015

Do not merge now, I will rename class_prefix to old_class_prefix and new_class_prefix to class_prefix

@peckpeck
Copy link
Member Author

peckpeck commented Sep 8, 2015

PR updated

@peckpeck peckpeck force-pushed the arch_7156/modify_generic_methods_to_define_and_use_the_new_class_prefix branch from 57b6d24 to 0279d43 Compare September 8, 2015 10:06
methods:
"placeholder" usebundle => file_copy_from_local_source_recursion("${source}", "${destination}", "0");
"copy without recursion" usebundle => file_copy_from_local_source_recursion("${source}", "${destination}", "0");
"new result classes" usebundle => _classes_copy("${class_prefix}_copy_without_recursion", "${class_prefix}");
Copy link
Member

Choose a reason for hiding this comment

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

shoudln't this call a logger ?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, because the sub method already does it.
The logger refactoring will occur in a future PR.

Copy link
Member

Choose a reason for hiding this comment

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

oh, ok !

@peckpeck
Copy link
Member Author

peckpeck commented Sep 9, 2015

PR updated

@peckpeck peckpeck force-pushed the arch_7156/modify_generic_methods_to_define_and_use_the_new_class_prefix branch from 0279d43 to bde51e4 Compare September 9, 2015 09:12
# WARNING !!!
# For new class prefix migration only, this will be removed when migration is over
# Define x and y prefixed/suffixed with promise outcome
body classes classes_generic_two(x,y)
Copy link
Member

Choose a reason for hiding this comment

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

x and y are really bad names :(

Copy link
Member Author

Choose a reason for hiding this comment

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

This is merely a copy of the stdlib code.

@peckpeck peckpeck force-pushed the arch_7156/modify_generic_methods_to_define_and_use_the_new_class_prefix branch from bde51e4 to 61d9dd0 Compare September 9, 2015 12:27
@peckpeck peckpeck force-pushed the arch_7156/modify_generic_methods_to_define_and_use_the_new_class_prefix branch from 61d9dd0 to 3a9cc2d Compare September 9, 2015 12:46
ncharles added a commit that referenced this pull request Sep 9, 2015
…s_to_define_and_use_the_new_class_prefix

Fixes #7156: Modify generic methods to define and use the new class_prefix
@ncharles ncharles merged commit 8a19a6c into Normation:master Sep 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants