Git module refactor and new git clean files feature #2980
Conversation
|
The refactoring touches code from the following commits that I got from git blame, might be helpful in reviewing the changes : |
raags
changed the title
|
Thanks @raags for this PR. This module is maintained by the Ansible core team, so it can take a while for patches to be reviewed. Thanks for your patience. |
|
Thanks @raags for this PR. Unfortunately, it is not mergeable in its current state due to merge conflicts. Please rebase your PR. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review. [This message brought to you by your friendly Ansibull-bot.] |
|
ready_for_review |
|
Thanks @raags for this PR. This module is maintained by the Ansible core team, so it can take a while for patches to be reviewed. Thanks for your patience. [This message brought to you by your friendly Ansibull-bot.] |
|
Thanks @raags for this PR. Unfortunately, it is not mergeable in its current state due to merge conflicts. Please rebase your PR. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review. [This message brought to you by your friendly Ansibull-bot.] |
raags
force-pushed
the
devel
branch
2 times, most recently
from
e508e3c
to
64bf5f6
Compare
|
Thanks @raags for this PR. This module is maintained by the Ansible core team, so it can take a while for patches to be reviewed. Thanks for your patience. Core team: please review according to guidelines (http://docs.ansible.com/ansible/developing_modules.html#module-checklist) and comment with 'needs_revision' or merge as appropriate. [This message brought to you by your friendly Ansibull-bot.] |
| choices: [ "yes", "no" ] | ||
| version_added: "2.1" | ||
| description: | ||
| - If C(yes), clean untracked files and directories in the repository |
There was a problem hiding this comment.
I think a cleaner interface is an option clean: None|ignored|untracked|all
There was a problem hiding this comment.
Yea, that looks much cleaner, I'll make the changes.
|
there are a bunch of unrelated style changes, this is not OK as it changes attribution for no good reason. |
|
@bcoca imho trailing spaces shouldn't be accepted, its sloppy. I removed dead code, and changed a non pythonic check that pep complained about. I kept it a separate commit which makes it clear its minor formatting change. note: closed the pr by mistake, and reopened it which remoed the needs_revision tag |
* clean_ignored and clean_untracked options will clean ignored and untracked file respectively. * Added untracked_files and ignored_files keys to the output, and changed local_mods key to local_mod_files to keep it consistent.
|
@bcoca I've removed the formatting changes, please review. ready_for_review |
|
Thanks @raags for this PR. This module is maintained by the Ansible core team, so it can take a while for patches to be reviewed. Thanks for your patience. Core team: please review according to guidelines (http://docs.ansible.com/ansible/developing_modules.html#module-checklist) and comment with 'needs_revision' or merge as appropriate. [This message brought to you by your friendly Ansibull-bot.] |
|
@raags why did you put the Independent of when clean is run an integration test of |
|
@robinro |
|
cleaning can interfere with switch_version if the file is not tracked in the current branch, but tracked in the other branch; if cleaned first, the version switch will work without force, but break if clean is run afterwards |
|
Thanks @raags for this PR. Unfortunately, it is not mergeable in its current state due to merge conflicts. Please rebase your PR. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review. [This message brought to you by your friendly Ansibull-bot.] |
|
@raags A friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open pending your changes. When you do address the issues, please respond with ready_for_review in your comment, so that we can notify the maintainer. [This message brought to you by your friendly Ansibull-bot.] |
|
@raags Another friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open. If you have addressed the issues and believe it's ready for review, please comment with the text "ready_for_review". If we don't hear from you within another 14 days, we will close this pull request. [This message brought to you by your friendly Ansibull-bot.] |
|
@raags A friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open pending your changes. When you do address the issues, please respond with ready_for_review in your comment, so that we can notify the maintainer. [This message brought to you by your friendly Ansibull-bot.] |
|
@raags Another friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open. If you have addressed the issues and believe it's ready for review, please comment with the text "ready_for_review". If we don't hear from you within another 14 days, we will close this pull request. [This message brought to you by your friendly Ansibull-bot.] |
|
@raags A friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open pending your changes. When you do address the issues, please respond with ready_for_review in your comment, so that we can notify the maintainer. [This message brought to you by your friendly Ansibull-bot.] |
|
@raags Another friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open. If you have addressed the issues and believe it's ready for review, please comment with the text "ready_for_review". If we don't hear from you within another 14 days, we will close this pull request. [This message brought to you by your friendly Ansibull-bot.] |
|
@raags A friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open pending your changes. When you do address the issues, please respond with ready_for_review in your comment, so that we can notify the maintainer. [This message brought to you by your friendly Ansibull-bot.] |
|
@raags Another friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open. If you have addressed the issues and believe it's ready for review, please comment with the text "ready_for_review". If we don't hear from you within another 14 days, we will close this pull request. [This message brought to you by your friendly Ansibull-bot.] |
|
@raags A friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open pending your changes. When you do address the issues, please respond with ready_for_review in your comment, so that we can notify the maintainer. [This message brought to you by your friendly Ansibull-bot.] |
|
@raags Another friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open. If you have addressed the issues and believe it's ready for review, please comment with the text "ready_for_review". If we don't hear from you within another 14 days, we will close this pull request. [This message brought to you by your friendly Ansibull-bot.] |
|
@raags A friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open pending your changes. When you do address the issues, please respond with ready_for_review in your comment, so that we can notify the maintainer. [This message brought to you by your friendly Ansibull-bot.] |
|
This repository has been locked. All new issues and pull requests should be filed in https://github.com/ansible/ansible Please read through the repomerge page in the dev guide. The guide contains links to tools which automatically move your issue or pull request to the ansible/ansible repo. |
|
This repository has been locked. All new issues and pull requests should be filed in https://github.com/ansible/ansible Please read through the repomerge page in the dev guide. The guide contains links to tools which automatically move your issue or pull request to the ansible/ansible repo. |
There are three commits with each one doing the following :
I initially just wanted to add the clean files feature, but it was cumbersome to do that without making these additional changes. I've run the integration tests against these changes, and I've also added more, which I'll push to the ansible repository now.