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

Proposed solution for Issue #22 App upgrade/update command #52

Closed
wants to merge 4 commits into from
Closed

Proposed solution for Issue #22 App upgrade/update command #52

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9cba3b2
initiate drush valkyrie upgrade
RealLukeMartin Mar 22, 2015
a953e9b
add git commands
RealLukeMartin Mar 23, 2015
c256fa6
check for correct global valkyrie path before running
RealLukeMartin Mar 23, 2015
1d8c76d
refactor and clean up
RealLukeMartin Mar 23, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion valkyrie.drush.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,11 @@ function valkyrie_drush_command() {
'aliases' => array('vup'),
'bootstrap' => DRUSH_BOOTSTRAP_DRUSH, // No bootstrap at all.
);

$items['valkyrie-upgrade'] = array(
Copy link
Member

Choose a reason for hiding this comment

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

because we're doing lots of other updating and upgrading, I'd suggest 'valkyrie-self-upgrade' or '...-self-update'.

'description' => 'Upgrade global Valkyrie extension and all of the submodules.',
Copy link
Member

Choose a reason for hiding this comment

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

no need to mention submodules

'aliases' => array('vug'),
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 we need an alias. In fact, it might be a good idea to force someone to type it out, since it is potentially destructive.

Copy link
Member

Choose a reason for hiding this comment

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

by default, this should probably be equivalent to 'drush dl valkyrie'. That is, download and install the release tarball. Maybe add a --git option, to optionally switch to, or update a git version.

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 one is a little ambiguous to me, needing some clearing up on it.

If we need to download the latest release tarball, I feel like we will need something like drush pm-releases valkyrie or an equivalent hook to give us the title of the latest release and pop it at the end of http://ftp.drupal.org/files/projects/valkyrie- and then add .tar.gz after that.
Right now drush pm-releases valkyrie doesn't show any releases for Valkyrie as of yet, and It seems like we are reinventing drush dl with this as well.

Possibly I overcomplicated this, and we are going to explicitly call drush dl valkyrie when drush valkyrie-self-upgrade is ran, and just overwrite the old project, but that seems redundant to me here.

Copy link
Member

Choose a reason for hiding this comment

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

The default drush valkyrie-self-upgrade behaviour should simply be to call drush dl. Presumably, most people will install from the release (maybe via a one-liner, see #76), so upgrading to the current release is the most reasonable expectation. And yes, it is somewhat redundant. However, it would give us an opportunity to check on Vagrant and Virtualbox versions, for example, if we up those in a future release. I'm not suggesting that we do that now, but it's something to consider.

It also allows us to switch to git, which is the point of #22. This'll allow users to easily test feature branches, bug fixes, or just live on the bleeding edge. However, once we're using the git version, we should be able to switch back to releases, if more stability is desired. We can check for the presence of .git/ to determine whether the default should be drush dl valkyrie, or git pull && git submodule update --recursive.

Eventually, it might be nice to provide an --update-projects option, that would look at the machine output of vagrant global-status and update all the valkyrie-based projects that it finds automatically.

Copy link
Member Author

Choose a reason for hiding this comment

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

Great explanation this clears it up 100% for me.

Thanks!

'bootstrap' => DRUSH_BOOTSTRAP_DRUSH, // No bootstrap at all.
);
$items['valkyrie-platform-add'] = array(
'description' => 'Register an existing platform with Valkyrie',
'arguments' => array(
Expand Down Expand Up @@ -346,6 +350,20 @@ function drush_valkyrie_update() {
}
}
}
function drush_valkyrie_upgrade() {
$current_path = getcwd();
$src = dirname(__FILE__);
if($src != $current_path) {
drush_print('You need to be in (~/.drush/valkyrie) to do this');
Copy link
Member

Choose a reason for hiding this comment

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

check out drush_cd_and_exec(), instead of this. Alternatively, if we really want the git pull below to display in real-time, we can always just chdir(dirname(__FILE__))

}
else {
drush_print('Checking for updates...');
Copy link
Member

Choose a reason for hiding this comment

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

There are a number of things that we might want to do here, in addition. For example, ensure that we're on the correct branch. We might also want to add --rebase, gracefully handle errors, etc.

I'd suggest that we use drush_shell_exec() rather the drush_shell_proc_open() by default. Maybe check drush_get_option('debug', FALSE) to display the full output.

drush_shell_proc_open('git pull');
drush_print('Checking submodule updates...');
drush_shell_proc_open('git submodule update --init --recursive');
drush_log('Upgrade Finished', 'ok');
}
}

/**
* Command callback for the 'valkyrie-snapshot' command.
Expand Down