Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Added "g" flag to project path replacement #291

Merged
merged 5 commits into from
Nov 30, 2016
Merged

Added "g" flag to project path replacement #291

merged 5 commits into from
Nov 30, 2016

Conversation

hiroaki-yamamoto
Copy link
Contributor

@hiroaki-yamamoto hiroaki-yamamoto commented Nov 28, 2016

Hi,
I found multiple $PROJECT and $PROJECT_NAME were not replaced properly and this PR fixes the issue.

Edit (by @Arcanemagus):
Fixes #290.

@Arcanemagus
Copy link
Member

Can you please add a spec testing this?

@hiroaki-yamamoto
Copy link
Contributor Author

Done, but sorry, I created a new file that is named "utils.js" to test the replacement function.

@Arcanemagus
Copy link
Member

I meant adding a test like this one, but with multiple replacements 😕. As far as I can tell your new specs are just duplicating the existing ones for the most part.

Also I'd much prefer not creating a new file for a 2 line function, especially since what it does is already being tested in the executable path section.

@hiroaki-yamamoto
Copy link
Contributor Author

Done.

@Arcanemagus
Copy link
Member

Much cleaner, thanks for sticking through with this!

Copy link
Member

@Arcanemagus Arcanemagus left a comment

Choose a reason for hiding this comment

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

LGTM

@Arcanemagus Arcanemagus changed the title [Ref: #290] Added "g" Added "g" flag to project path replacement Nov 30, 2016
@Arcanemagus Arcanemagus merged commit d9644fb into AtomLinter:master Nov 30, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple $PROJECT not work
2 participants