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

Create --offline flag to disable git functionality #53

Merged
merged 1 commit into from
Aug 10, 2015
Merged

Create --offline flag to disable git functionality #53

merged 1 commit into from
Aug 10, 2015

Conversation

pabelanger
Copy link
Contributor

This allows existing cloned projects to be used for testing purposes.

Signed-off-by: Paul Belanger pabelanger@redhat.com

@igalic
Copy link
Contributor

igalic commented Aug 7, 2015

@pabelanger would you mind fixing the broken tests?

@cmurphy
Copy link
Contributor

cmurphy commented Aug 7, 2015

There will often be cases when you do want to clone the repos, you just don't want to push. Could we instead add a different flag to handle this?

@pabelanger
Copy link
Contributor Author

@cmurphy suggestions on flag name? --disable-git? --without-git?

@cmurphy
Copy link
Contributor

cmurphy commented Aug 7, 2015

--no-pull? --no-clone? --without-clone?

Maybe a --offline that neither pulls nor pushes?

@pabelanger
Copy link
Contributor Author

--offline works. Let me refactor

@cmurphy cmurphy changed the title When using --noop don't clone git repo Create --offline flag to disable git functionality Aug 7, 2015
@cmurphy
Copy link
Contributor

cmurphy commented Aug 7, 2015

Also @danzilio might want to take a look since he's working on a refactor

@@ -56,8 +56,10 @@ def self.run(args)
# managed_modules is either an array or a hash
managed_modules.each do |puppet_module, opts|
puts "Syncing #{puppet_module}"
git_base = "#{options[:git_base]}#{options[:namespace]}"
Git.pull(git_base, puppet_module, options[:branch], opts || {})
if not options[:offline]
Copy link
Member

Choose a reason for hiding this comment

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

a more idiomatic way to put this would be unless options[:offline]

@danzilio
Copy link
Member

danzilio commented Aug 7, 2015

The --offline option name makes sense to me! I agree with @cmurphy that this may not be desirable as the default behavior for --noop

@rnelson0
Copy link
Sponsor Member

rnelson0 commented Aug 7, 2015

Could some instructions for the new flag be added to https://github.com/puppet-community/modulesync#dry-run please?

@@ -78,6 +80,7 @@ def self.run(args)
end
end
files_to_manage -= files_to_delete
next if options[:offline]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we do still want the update_noop to happen in offline mode (it just prints the diffs to stdout)

This allows existing cloned projects to be used for testing purposes.

Signed-off-by: Paul Belanger <pabelanger@redhat.com>
@@ -80,7 +82,7 @@ def self.run(args)
files_to_manage -= files_to_delete
if options[:noop]
Git.update_noop(puppet_module, options)
else
elsif not options[:offline]
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this addresses #53 (comment)

I think line 83 should be if options[:noop] || options[:offline] since we want the same behavior (printing out the changes without making updates)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, if we do that, we're changing the way --noop actually works. Since now, we get output with both --noop or --offline.

This way, you have to pass both --offline --noop if you want to see the diff. Other wise, --offline does nothing related to git.

cmurphy added a commit that referenced this pull request Aug 10, 2015
Create --offline flag to disable git functionality
@cmurphy cmurphy merged commit 759a9ee into voxpupuli:master Aug 10, 2015
danzilio pushed a commit to danzilio/modulesync that referenced this pull request Aug 11, 2015
This adds the offline parameter from voxpupuli#53. This also adds some more
documentation to README.md.
openstack-gerrit pushed a commit to openstack-archive/puppet-modulesync-configs that referenced this pull request Aug 19, 2015
This is our hook from the propose_update.sh script. It is also
dependant on the following features upstream:

  voxpupuli/modulesync#53
  voxpupuli/modulesync#54

Change-Id: I8b9e7edd564e55eaba8ed95294bd185f0fe2a585
Signed-off-by: Paul Belanger <pabelanger@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants