shawnrice / alfred-bundler

a utility / workflow to handle dependencies
MIT License
21 stars 3 forks source link

AB_MAJOR_VERSION/environmental variable? #44

Closed deanishe closed 10 years ago

deanishe commented 10 years ago

What's the purpose of this code from the bash wrappers?

# Define the global bundler version.
if [ -f "../meta/version_major" ]; then
  declare AB_MAJOR_VERSION=$(cat "../meta/version_major")
else
  declare AB_MAJOR_VERSION="devel"
fi

I'm not clear on why you'd want to check a file that's only present if the wrapper is being run from the repo.

For development purposes, I think it would be a good idea to be able to set an environmental variable to specify which major version (i.e. branch) to use. That way, I can tinker on the Python bundler in a different branch without having to remember to change loads of files on merge.


Use Environmental Variable
shawnrice commented 10 years ago

It might be overkill as the wrappers should define the bundle version no matter what, and we'll always have to change those.

The idea was that the 'version_major' file should always be the most authoritative, but, sans its presence, the wrapper should consider itself the authority.

That logic probably came out of my head late at night when I was starting to use the bundler wrappers with each other.

It doesn't have to stay.

deanishe commented 10 years ago

The file strikes me as just another place the version needs to be updated.

It must be in 10 places already.

As a result, I think supporting an environmental version could help us avoid forgetting to update a file. I've corrected a couple of such errors today.

shawnrice commented 10 years ago

We should have to update it in six places, the five wrappers and version_major. If it's hard coded anywhere else, then that should be fixed. Remembering those six files shouldn't be too hard.

shawnrice commented 10 years ago

To follow up: all of the backends of the bundlers should be getting their version from that version_major file as it will be correct for the branch. If we don't use that file, then we'd have to define the variable in almost every file in the bundler, making it much easier to miss them on update.

The only place that the minor version should exist in the bundler is in the minor_version file. Ultimately, the only files that should care about the minor version are the update script and its children.

deanishe commented 10 years ago

That makes complete sense for the files within the bundler, but not imo, the wrappers, which will spend most of their lives living elsewhere. That's kinda their job.

I'd remove the code that looks for meta/version_major or, as noted, replace it with a test for a BUNDLER_MAJOR_VERSION environmental variable, which may make development a tad easier (i.e. I can point the wrappers at the branch I'm working on without having to edit the files and "pollute" the commit log with said edits).

For the same reason, I'd like some way to be able to point the wrapper to HEAD instead of "latest", which requires tagging a commit. Perhaps another environmental variable, e.g. BUNDLER_DIST_VERSION or BUNDLER_USE_HEAD. It's time-consuming enough working on install code (which requires you to push each change) without having to also tag every push.

It's not like this is a major amount of work: I'm mostly motivated by reducing the opportunities to make dumb oversights.

I can set one or two environmental variables, do my coding, and then forget about it, as the "changes" I made will disappear with the shell session. I don't have to remember to revert the changes I've made to source code. And then redo them when I create a new branch etc.

shawnrice commented 10 years ago

The idea of doing the tagged release is that (1) it'll always download the newest "minor" version of the bundler during installation, and (2) we can commit some things into an existing branch without screwing up others' new installations.

I know that we talked about workflow with more of a Gitflow model or using the dev branch more for testing, but I like the idea of tagging stable releases. However, it sucks for development purposes in which we're constantly deleting the bundler dir and re-installing.

How about a compromise: we comment out the "latest" tags in all the wrappers for now and just download from the equivalent of the head. Then, we we're ready to release, we do a "latest" tag. Good?

shawnrice commented 10 years ago

Or we can set the environmental variable that would be either "DEVELOPMENT" or "PRODUCTION"; I'm fine with whichever.

deanishe commented 10 years ago

Release branches absolutely should be tagged, but not development branches, imo. I don't see the need to be pointing at -latest in these branches.

Users should be installing from master, or possibly aries or taurus, and they should definitely be tagged. I must admit I'm not at all clear on the purpose of master and its relationship to the named version branches. I suppose production releases will have to point to the correspondingly-named branches, not master, as having an Aries install update the backend to Taurus would break shit.

Due to the way the bundler works, it's not possible to test it properly using local branches, and I think it's reasonable to expect that as a result, we may want to create other dev branches that we can push to GH to test out things we're working on without potentially borking the main devel branch for the other developers. And I'd like to make that as simple as possible.

Thus, I'd really like a way to be able to easily point the bundler at a different branch without having to edit a bunch of files (that's just so error prone). Then I can do, say, export BUNDLER_BRANCH=super-cool-feature in my shell (and possibly workflows), and hack away on my own branch of the same name without having to worry about changing half a dozen things across half a dozen files every time I want to merge it into devel, and possibly back again if I want to continue working on that branch.

So let's say the relevant files have a hard-coded AB_MAJOR_VERSION and an AB_DOWNLOAD_ENDPOINT equivalent to ${AB_MAJOR_VERSION}-latest. If however, the env var BUNDLER_BRANCH (or whatever) is set, AB_MAJOR_VERSION and AB_DOWNLOAD_ENDPOINT are both set to that, meaning that the bundler would now be pointing at HEAD in the BUNDLER_BRANCH branch.

Am I the only one that would find that useful?

shawnrice commented 10 years ago

Oh. That would be useful. I just didn't get it.

master is there because git creates that by default. I've since set devel as default, so master has absolutely no reason to exist. Should we stage an uprising and kill the master (branch)?

So, for implementation, we'd put in something only in the wrappers that would read the environmental variable? Or do you want to get away without having to query the version_major file?

If we're just trying to differentiate development, then we can do this:

declare AB_MAJOR_VERSION='devel'
declare AB_DEVELOPMENT='TRUE'
...
function AlfredBundler::install_bundler() {
  if [ "${AB_DEVELOPMENT}" != "TRUE" ]; then
      suffix="-latest"
  fi

 AB_BUNDLER_SERVERS=("https://github.com/shawnrice/alfred-bundler/archive/${AB_MAJOR_VERSION}${suffix}")
...
}

Otherwise, do you mean that you want the environmental variable to override version_major? If that's the case, then we can check to see if that variable is set, and, if it is, then we can just set the branch as that rather than follow the other logic.

shawnrice commented 10 years ago

09a07337eeed4a0d4f32411c72c3691229120b89 introduces an environmental variable ALFRED_BUNDLER_DEVEL into the Bash, Misc, PHP, and Ruby bundlers. That variable (set via export) will override any internal bundler setting. It seems to work so far, but we need to setup a test branch that has the same install logic in order to make sure.

shawnrice commented 10 years ago

b6a691818738d2aa226d5fb3c77820d3029b968d Sets the bundler servers to "head" when environmental variable ALFRED_BUNDLER_DEVEL is set / not empty. Change in Bash, Misc, and PHP wrappers.

Hence, if you just export that variable to devel, then it will (read: should) use the head.

shawnrice commented 10 years ago

@deanishe: Can you set this functionality in the Python bundler, then check off the last item and close the issue?

shawnrice commented 10 years ago

Spoke too soon on Ruby.

deanishe commented 10 years ago

Would we not need to also implement this where version_major is used to prevent the bundler getting confused over its version?

shawnrice commented 10 years ago

Depending on the implementation. We can set that through all the other files, or we could just change the contents of version_major per branch. We can do it either way.

I just did it wherever the variable was hardcoded.

deanishe commented 10 years ago

I created a new branch to fiddle with some caching features, and the implementation appears to be incomplete. I changed the PHP bundler to observe the env var, but I'm sure there are still a couple of places that need changing.

Moreover, I deeply dislike the name ALFRED_BUNDLER_DEVEL. It sounds like a True/False flag and is ugly.

I propose changing the name to AB_BRANCH. It's much shorter, and therefore easier to set, and much more descriptive of the purpose of the variable. It's also more inline with the names of other bundler variables.

shawnrice commented 10 years ago

Done. We'll use AB_BRACH. I'll see what I can do with it too.

shawnrice commented 10 years ago

So, I created a branch called devel-env, exported the environmental variable, and then I ran the misc wrapper then the PHP wrapper. Then I delete the data directories and ran them in random orders over and over again.

And.... everything works just fine with the current implementation (after your edits, Dean, mind you). Everything installs correctly. Everything loads correctly. The call-cache is populated, the color-cache is populated. The assets are loaded... So, the environmental variable in at least the PHP and Misc bundlers work just fine.

deanishe commented 10 years ago

I wasn't seeing the notifications and dialogs. In hindsight, the lack of notifications was likely due to my coding into the Quiet Hours (that gets me every single time).

shawnrice commented 10 years ago

This is implemented. Closing.