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.

Sign up for GitHub

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

Jump to bottom

Fixes #12096: Rename some generic methods to match our naming convention - part2 #701

Merged
merged 4 commits into from
Feb 28, 2018
Merged

Fixes #12096: Rename some generic methods to match our naming convention - part2 #701

merged 4 commits into from
Feb 28, 2018

Conversation

amousset
Copy link
Member

@amousset amousset commented Feb 8, 2018

@amousset
Copy link
Member Author

amousset commented Feb 8, 2018

Do not merge, tests not passing

@amousset
Copy link
Member Author

amousset commented Feb 9, 2018

Commit modified

@amousset amousset force-pushed the ust_12096/rename_some_generic_methods_to_match_our_naming_convention_part2 branch from dd4ef51 to 7ebf337 Compare February 9, 2018 11:58
@amousset
Copy link
Member Author

amousset commented Feb 9, 2018

Commit modified

@amousset amousset force-pushed the ust_12096/rename_some_generic_methods_to_match_our_naming_convention_part2 branch from 7ebf337 to a95254e Compare February 9, 2018 15:28
@amousset
Copy link
Member Author

amousset commented Feb 9, 2018

Commit modified

@amousset amousset force-pushed the ust_12096/rename_some_generic_methods_to_match_our_naming_convention_part2 branch from a95254e to cdafaa6 Compare February 9, 2018 15:38
@amousset
Copy link
Member Author

amousset commented Feb 9, 2018

All tests fixed, ready for review!

@amousset
Copy link
Member Author

amousset commented Feb 9, 2018

Commit modified

@amousset amousset force-pushed the ust_12096/rename_some_generic_methods_to_match_our_naming_convention_part2 branch from cdafaa6 to 4bb7e92 Compare February 9, 2018 15:51
peckpeck
peckpeck requested changes Feb 14, 2018
View reviewed changes
Copy link
Member

@peckpeck peckpeck left a comment

Choose a reason for hiding this comment

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

You need to review friendly names.
Everything else looks good to me

tree/30_generic_methods/file_content.cf
@@ -23,13 +23,13 @@
# @parameter lines Line(s) to add in the file
# @parameter enforce Enforce the file to contain only line(s) defined (true or false)
#
# @class_prefix file_content
# @class_prefix file_lines_present
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should change the prefix of existing generic methods

Copy link
Member Author

Choose a reason for hiding this comment

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

It was wrongly changed in the first rename PR.

Copy link
Member

Choose a reason for hiding this comment

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

OK

tree/30_generic_methods/file_keys_values_present.cf Outdated
#
#####################################################################################

# @name File ensure keys -> values present
Copy link
Member

Choose a reason for hiding this comment

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

You could rename the friendly name too

tree/30_generic_methods/file_line_present_in_ini_section.cf Outdated
#
#####################################################################################

# @name File ensure line in INI section
Copy link
Member

Choose a reason for hiding this comment

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

Here too

@amousset
Copy link
Member Author

Commit modified

@amousset amousset force-pushed the ust_12096/rename_some_generic_methods_to_match_our_naming_convention_part2 branch from 4bb7e92 to 7537972 Compare February 14, 2018 09:00
peckpeck
peckpeck requested changes Feb 14, 2018
View reviewed changes
tree/30_generic_methods/$ Outdated
@@ -0,0 +1,42 @@
#####################################################################################
Copy link
Member

Choose a reason for hiding this comment

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

What is this file name ?

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 idea where is comes from...

tree/30_generic_methods/file_from_local_source_recursion.cf Outdated
#
#####################################################################################

# @name File from local source recurse
Copy link
Member

Choose a reason for hiding this comment

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

recursion

tree/30_generic_methods/file_from_remote_source_recursion.cf Outdated
#
#####################################################################################

# @name File from remote source recurse
Copy link
Member

Choose a reason for hiding this comment

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

recursion

tree/30_generic_methods/file_key_value_present.cf Outdated
#
#####################################################################################

# @name File key -> value present
Copy link
Member

Choose a reason for hiding this comment

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

Why the '->' here and not in other names ?

tree/30_generic_methods/file_key_value_present_in_ini_section.cf
#
#####################################################################################

# @name File key-value in INI section
Copy link
Member

Choose a reason for hiding this comment

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

'-' here

Copy link
Member Author

Choose a reason for hiding this comment

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

I put "-" everywhere as it is clearer than "key value" and shorter than "key -> value"

tree/30_generic_methods/file_symlink_present.cf Outdated
#
#####################################################################################

# @name Create symlink
Copy link
Member

Choose a reason for hiding this comment

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

Create ?

@amousset
Copy link
Member Author

Commit modified

@amousset amousset force-pushed the ust_12096/rename_some_generic_methods_to_match_our_naming_convention_part2 branch from 7537972 to 082e39a Compare February 14, 2018 09:32
@amousset
Copy link
Member Author

Commit modified

@amousset amousset force-pushed the ust_12096/rename_some_generic_methods_to_match_our_naming_convention_part2 branch from 082e39a to c1b105e Compare February 15, 2018 12:11
@amousset
Copy link
Member Author

Commit modified

@amousset amousset force-pushed the ust_12096/rename_some_generic_methods_to_match_our_naming_convention_part2 branch from c1b105e to 8232866 Compare February 15, 2018 13:06
@amousset
Copy link
Member Author

ping @peckpeck

peckpeck
peckpeck approved these changes Feb 28, 2018
View reviewed changes
Copy link
Member

@peckpeck peckpeck left a comment

Choose a reason for hiding this comment

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

Very good

@amousset
Copy link
Member Author

OK, merging this PR

@amousset
Copy link
Member Author

Commit modified

@amousset amousset force-pushed the ust_12096/rename_some_generic_methods_to_match_our_naming_convention_part2 branch from 8232866 to 1923cf9 Compare February 28, 2018 09:29
@amousset
Copy link
Member Author

OK, merging this PR

@amousset
Copy link
Member Author

Commit modified

@amousset amousset force-pushed the ust_12096/rename_some_generic_methods_to_match_our_naming_convention_part2 branch from 1923cf9 to 2437223 Compare February 28, 2018 09:44
@amousset
Copy link
Member Author

OK, merging this PR

@amousset amousset merged commit 2437223 into Normation:branches/rudder/4.3 Feb 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peckpeck peckpeck peckpeck approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@amousset @peckpeck