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

Add git branch finder and test #14

Merged
merged 13 commits into from
Mar 9, 2013
Merged

Add git branch finder and test #14

merged 13 commits into from
Mar 9, 2013

Conversation

alexcouper
Copy link
Member

@txels

This is now a full-blown PR

'GitPlugin': {
'code_dir': None
}
}
Copy link
Member

Choose a reason for hiding this comment

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

What about having the config in the plugin list? As in plugin list contains tuples with class name (str) and config (dict).
BTW what made you keep all the separate lists of plugins, instead of a single data structure?

Copy link
Member Author

Choose a reason for hiding this comment

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

The motivation behind the separate lists was to do with attempting to make customizing configuration easier. But actually, it only reduces the pain without solving it.

But since we're thinking about db-driven per-user configuration this is probably a non-issue now.

@txels
Copy link
Member

txels commented Mar 3, 2013

Re: your question - what about creating a repo in github for the sole purpose of using it in deploystream tests? E.g. deploystream-test-repo. As long as we only clone it and never push to it we are fine I guess, any people could run any number of tests in parallel.

But if we want to push to it, I guess the github API supports creation of repos as well. You can create one with a random name.

@alexcouper
Copy link
Member Author

I've created a dummy repo - I'll use that in the tests. For now there's no pushing to the repo anyway.



SOURCE_CODE_PLUGINS = [
('deploystream.providers.git_plugin.plugin.GitPlugin', {}),
('deploystream.providers.git_provider.GitProvider', GIT_CONFIG),
Copy link
Member Author

Choose a reason for hiding this comment

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

The reason for the git_provider name rather than git is because git is also the name of a library that we use.

Copy link
Member

Choose a reason for hiding this comment

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

yeah I remember that problem from gdfabric and storyboard

@alexcouper
Copy link
Member Author

@txels: this is ready for review now...

@@ -20,7 +23,17 @@ def get_repo_branches_involved(self, feature_id):
1: branch name
2: latest commit
"""
return [] # no info available for now
# Every folder in side self.code_dir that is a git repo will be looked
Copy link
Member

Choose a reason for hiding this comment

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

inside?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@txels
Copy link
Member

txels commented Mar 7, 2013

out

@alexcouper
Copy link
Member Author

I've added the regexing.

If this doesn't cover a use case that you are thinking of then perhaps we should have a chat about it?

@txels
Copy link
Member

txels commented Mar 8, 2013

OK maybe we should chat or I should add some example, because what you did is very different from what I had in mind.

@alexcouper
Copy link
Member Author

I'm up for chatting about it tomorrow (sat) morning. Does that work for you?
On 8 Mar 2013 22:55, "Carles Barrobés i Meix" [email protected]
wrote:

OK maybe we should chat or I should add some example, because what you did
is very different from what I had in mind.


Reply to this email directly or view it on GitHubhttps://github.com//pull/14#issuecomment-14651018
.

@alexcouper
Copy link
Member Author

OK, well seems like you're not around online at the moment and I need to head off shortly.

At the moment the core code calls get_repo_branches_involved on a VCS provider and at that point only knows the feature id. The purpose of this is so that we can get back all the information to do with a feature. I'm unsure then why we need to worry about staging, or develop or users or anything else.

So a question is "who should own the breaking up of a feature id into parts that can be matched in branch names". And I guess that could live up in the core code, and then the get_repo_branches_involved could instead take as arguments the keywords used in the breaking up code.
eg.

# Config
FEATURE_BREAKUP = "(?P<project>[a-zA-Z]+)-?(?P<id>[0-9]+)" 

# Core code
match = re.search(config.FEATURE_BREAKUP, feature_id)
vcs_plugin.get_repo_branches_involved(**match.groupdict())

# VCS provider
def get_repo_branches_involved(self, **kwargs):
    # kwargs contains project and id keys in this case
    regex_matcher = self.template.format(**kwargs)
    # ...

and then the provider itself is configured with the template regex for branch finding. eg ".*{project}{id}.*". In this way retrieving would be configurable and decoupled from the process logic.

But I think fundamentally we need to get on the same page over which part of the system is responsible for what - and I'm very unclear as to the purpose of your desire for staging, develop or name/ matching.

@txels
Copy link
Member

txels commented Mar 9, 2013

That is the gist of it. The VCS provider code should not know about features, only branches.

In the scheme I had in mind, an upper layer would call it only with a list of strings to match for a specific case. I would make it look like:

# VCS 
def get_branches(self, regexes=None):
    """
    regexes == None - return all branches
    regexes == list of strings: branches whose name matches any of the regexes
    """
    ...

regexes could be a list, or a single regex using | (I think the list would be more useful later, as we could use it to express branch hierarchy when looking for things like branches that are [un]merged on the way up [e.g. th way up for GD would be user_dev -> story -> rc -> staging -> master, somewhere else this could be story_branch -> develop -> master, etc]).

On a specific case (story 347) it could get called with:

regexes = ["^staging$", "^rc_us347", "^us347", "^us347.*_dev_.*"]  # GD
regexes = ["^develop$", "^\w+/347-.*"]  # RBX

The concrete list of matchers would be built by a higher level, not the VCS plugin itself, where project rules or conventions (such as branch naming and structure, etc) would be defined.

I do not mean this is how it should be, just how I was thinking it would end up being.

@txels
Copy link
Member

txels commented Mar 9, 2013

BTW I'm happy to merge this as it is, I think we will figure out the right APIs when implementing end to end stories (as happened with feature list). Let's define an end-to-end story that uses this code.

txels added a commit that referenced this pull request Mar 9, 2013
Add git branch finder and test
@txels txels merged commit ddd8588 into develop Mar 9, 2013
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