Skip to content
This repository has been archived by the owner on Jan 21, 2020. It is now read-only.

Added installer option to remove composer.lock file from .gitignore file #88

Merged
merged 2 commits into from
Apr 21, 2016
Merged

Added installer option to remove composer.lock file from .gitignore file #88

merged 2 commits into from
Apr 21, 2016

Conversation

jonsa
Copy link

@jonsa jonsa commented Apr 18, 2016

Fixes #87

* @param IOInterface $io
* @return bool
*/
private static function requestExcludeComposerLock(IOInterface $io)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this needs to be a question. Best practice is to commit the lock file so it shouldn't be optional.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed.

@jonsa
Copy link
Author

jonsa commented Apr 21, 2016

@weierophinney anything else you'd like to see in this pull request?

* @param string $content String to remove entry from.
* @return string
*/
public static function removeLine($entry, $content)
Copy link
Member

Choose a reason for hiding this comment

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

Let's rename this to removeLineFromFile().

Copy link
Author

Choose a reason for hiding this comment

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

It did at first, but it didn't feel right since it isn't necessarily a file that is passed to it. But I can change it back.

Copy link
Member

Choose a reason for hiding this comment

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

I missed that. Okay, let's change it to removeLineFromString(), then.

@weierophinney weierophinney self-assigned this Apr 21, 2016
@weierophinney weierophinney added this to the 1.0.2 milestone Apr 21, 2016
@weierophinney
Copy link
Member

@jonsa I can make the suggested changes now, if you'd like; I'm working on some other updates for a 1.0.2 release today.

@jonsa
Copy link
Author

jonsa commented Apr 21, 2016

@weierophinney That's fine by me.

@weierophinney weierophinney merged commit e31c53f into zendframework:master Apr 21, 2016
weierophinney added a commit that referenced this pull request Apr 21, 2016
…itignore

Added installer option to remove composer.lock file from .gitignore file
weierophinney added a commit that referenced this pull request Apr 21, 2016
- Renames new method to `removeLineFromstring()`
- Minor improvements to regex to ensure it only matches a line
  containing the given content (previous could match a partial line).
weierophinney added a commit that referenced this pull request Apr 21, 2016
weierophinney added a commit that referenced this pull request Apr 21, 2016
weierophinney added a commit that referenced this pull request Apr 21, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants