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

Include version when matching service name #164

Merged
merged 2 commits into from
Apr 2, 2019

Conversation

gscho
Copy link
Contributor

@gscho gscho commented Mar 11, 2019

Signed-off-by: gscho greg.c.schofield@gmail.com

Description

The current regular expression matches on service name which causes an issue when another service contains matches.

Ex: I have a loaded service named jenkins-slave-provisioner which uses the habitat cookbook to load up a jenkins-slave package. Since the current regex matches jenkins-slave-provisioner the jenkins-slave package is never loaded.

I've wrapped the regular expression in %r{} and added a / to the end of the pattern. I've also included the version in the source string built from ['spec_ident'] values.

Issues Resolved

#134

Check List

Signed-off-by: gscho <greg.c.schofield@gmail.com>
@gscho gscho changed the title Use %r to match service name Include version when matching service name Mar 11, 2019
@gscho
Copy link
Contributor Author

gscho commented Mar 12, 2019

Is there any reason why we need regex in this function? Is there any case where they wont be equal?

@gscho
Copy link
Contributor Author

gscho commented Mar 18, 2019

@jonlives Mind taking a peek at this when you have a time?

@jonlives
Copy link
Contributor

@gscho looks good to me - would be good to add some tests to verify that this fixes the stated problem though.

Signed-off-by: gscho <greg.c.schofield@gmail.com>
@jonlives jonlives merged commit db71b4a into chef-boneyard:master Apr 2, 2019
chef-ci added a commit that referenced this pull request Apr 2, 2019
Obvious fix; these changes are the result of automation not creative thinking.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants