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

Fixing regex for dirnames capitalized #22

Merged
merged 2 commits into from
May 9, 2019
Merged

Fixing regex for dirnames capitalized #22

merged 2 commits into from
May 9, 2019

Conversation

alvadorn
Copy link
Contributor

What does this PR do?

  • Fix regex on directory names capitalized

Like referenced on issue #19, the latest release of plis can't attach containers on directories which its name are capitalized, for example:

  • BACK-END-3, docker-compose uses back-end-3 as namespace but the current regex looks for BACK-END-3

@gabrielsclimaco
Copy link

+1

@vovimayhem vovimayhem self-requested a review May 9, 2019 15:46
Copy link
Contributor

@vovimayhem vovimayhem left a comment

Choose a reason for hiding this comment

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

Hi @alvadorn ! Thanks for your interest in plis :)

For the sake of clarity and purpose of the variables used in this method, I propose the following changes to the code in your PR:

1: leave dirName as it was prior to this PR:

            dirName := filepath.Base(currentWorkdir)

2: Add a new variable containerName:

        containerName := strings.ToLower(dirName + "_" + serviceName)

3: Use containerName in the regexPattern: (My bad... rp stands for "Regex Pattern" here... will correct that in another PR!)

      rp := regexp.MustCompile("^" + containerName + "_\\d+") 

Thanks for your PR! I'll just use a Willem Dafoe gif 'cause it's soo damn friggin' cool:
Keep em coming

@alvadorn
Copy link
Contributor Author

alvadorn commented May 9, 2019

Totally agreed with your review. Fix pushed.

@vovimayhem vovimayhem merged commit 8071966 into IcaliaLabs:master May 9, 2019
@vovimayhem
Copy link
Contributor

Thanks @alvadorn ! I'll try to cut a release tomorrow!

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.

3 participants