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

state server upgrade sync refactoring #732

Merged
merged 2 commits into from Sep 15, 2014

Conversation

mjs
Copy link

@mjs mjs commented Sep 10, 2014

Various clean ups for the recent state server upgrade synchronisation work.

@@ -172,131 +192,84 @@ func (c *upgradeWorkerContext) run(stop <-chan struct{}) error {
return nil
}

func (c *upgradeWorkerContext) initTag(tag names.Tag) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

why not change the type of the argument to names.MachineTag then it's the callers problem to pass the right value

Copy link
Author

Choose a reason for hiding this comment

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

Hmm... the main point of the method is the type assertion...

@wallyworld
Copy link
Member

I like the refactoring into separate methods. LGTM

Menno Smits added 2 commits September 15, 2014 12:24
1. Use version.Number's in logs and status output throughout instead
of version.Binary's. The series and architecture is irrelevant to the
upgrade process and including it just adds unnecessary verbosity.

2. The check for an already completed upgrade has been moved earlier
to avoid unnecessary initialisation.

3. New upgradeWorkerContext attributes have been added to make the
from version more accessible and the target version shorter to read
and write.
This revision is primarily about splitting up runUpgrades into smaller
pieces. This function was getting to be too large and difficult to
follow.
@mjs
Copy link
Author

mjs commented Sep 15, 2014

$$merge$$

@jujubot
Copy link
Collaborator

jujubot commented Sep 15, 2014

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

jujubot added a commit that referenced this pull request Sep 15, 2014
state server upgrade sync refactoring

Various clean ups for the recent state server upgrade synchronisation work.
@jujubot jujubot merged commit 48ec61c into juju:master Sep 15, 2014
@mjs mjs deleted the upgrade-sync-refactoring branch September 15, 2014 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants