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

Working rudimentary example version #2

Closed
wants to merge 10 commits into from
Closed

Working rudimentary example version #2

wants to merge 10 commits into from

Conversation

darrenmce
Copy link
Member

WIP: @mpareja something to work from at least, may be a while before we revisit so wanted to put this up here

@darrenmce
Copy link
Member Author

@mpareja i'm currently using this version, i'll continue to update this branch as i iron out the bugs

@mpareja
Copy link
Member

mpareja commented Dec 23, 2016

It looks like you're adding a workspace configuration model. If this is the case, why not remove the tons of code introduced by the projects/statics docker files and just have a variable containing the list of services which are considered "projects" rather than "static"? @darrenmce

@darrenmce
Copy link
Member Author

im leaning towards going back to the single docker-compose yml file for a multitude of reasons (actually using this is helping! we gotta get more people pre-alphaing this lol)

this is a much bigger change, and if ttvars ends up being a good spot for it, im all for it

ill make an issue so we can discuss that one directly

@darrenmce
Copy link
Member Author

darrenmce commented Dec 23, 2016

also, can we just merge this in? i dont think master has any value in its state, whereas im actually using this

edit: don't merge yet though, few more commits coming, just want thoughts

@mpareja
Copy link
Member

mpareja commented Dec 23, 2016

This PR has a bunch of different things going on, it appears. Is it possible to rebase this so we split the multiple DC files changes away from the other goodness on this branch? The version in master relies on the NOT_SERVICES_PATTERN variable. If you added ttvars on top of that, you could refactor to expect that variable to potentially be defined in ttvars. (Obviously, we probably want to rename it and or switch to the more positive corollary.)

I suppose what I'm saying is that this branch is a spike. Some things are turning out for the better and some things not. Can we create some new PRs that pull in each piece of functionality in a more iterative manner that we can be confident in?

#!/usr/bin/env bash

## TT_PROJECTS - This is the root folder of your git-backed projects
TT_PROJECTS=~/tmp
Copy link
Member

Choose a reason for hiding this comment

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

This is really cool. I suppose this would give you the option to both:

  1. create two workspaces that share the same source code directories for equally named services
  2. create two workspaces that do not share the same source code directories for equally named services

@darrenmce
Copy link
Member Author

darrenmce commented Dec 23, 2016

I think master is so extremely different, that i was thinking this could just replace master with a new starting point. could this branch be better? heck ya.. but out of the two (master and v0_spike), id rather see v0_spike in as the master branch for now.

Though i completely agree with your comment and that i'd love to see all that done, sadly i don't see it actually getting done in a reasonable amount of time :(

edit: not being able to tt upgrade suckz!

tt
@@ -57,6 +55,10 @@ main() {
esac
fi

TT_PROJECTS=${TT_PROJECTS:-~/tmp}
TT_GIT_BASE=${TT_GIT_BASE:[email protected]:meet-tarantino}
apply_workspace_vars # this may override the above defaults
Copy link
Member

Choose a reason for hiding this comment

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

Now that I think of it, are you sure we need to do this explicitly? Part of our vision included allowing workspaces to define their own bash scripts to be automatically sourced so that additional commands could be added. (Think tt logs being added by a workspace where Kibana is included.)

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, not sure if lines 58/59 need to exist, it was more for back-compat for my other workspace that uses those lol

Copy link
Member

Choose a reason for hiding this comment

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

Based on what's in master and #1, our intent was to allow workspaces to contain a plugins directory. We would then . source the scripts in there so we could inherit other tt_... functions. We could leverage that for configuration too?

Copy link
Member Author

Choose a reason for hiding this comment

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

interesting, yeah i remember that now, lets cross that bridge when we get to plugins :)

@mpareja
Copy link
Member

mpareja commented Dec 23, 2016

I'll take on splitting out commits. I'll even put you in as the commit author to make sure you get credit for your fine work 😃

@darrenmce
Copy link
Member Author

sweet, ok wait for my next set of commits (or be aware of them anyways) they are in workspace.sh

@mpareja
Copy link
Member

mpareja commented Dec 23, 2016

Cool, I'm going to start pumping PRs. Care to review and merge?

@darrenmce
Copy link
Member Author

will do, likely wont get to it til tonight or the weekend though

@mpareja
Copy link
Member

mpareja commented Jan 16, 2017

It looks like the only changes we're missing in the master branch now are some of the minor improvements to workspace.sh. Agree @darrenmce ?

@darrenmce
Copy link
Member Author

yeah I have a branch off master I already started, I'll dust it off next opportunity i get and hopefully put this spike to rest..

@darrenmce darrenmce mentioned this pull request Jan 30, 2017
@darrenmce
Copy link
Member Author

and... 14 days later the PR is up lol #21

closing this.

@darrenmce darrenmce closed this Jan 30, 2017
@darrenmce darrenmce deleted the v0_spike branch March 9, 2018 19:25
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.

2 participants