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 config flags.logLevel #113

Merged
merged 3 commits into from
Jun 7, 2017
Merged

Add config flags.logLevel #113

merged 3 commits into from
Jun 7, 2017

Conversation

sttk
Copy link
Contributor

@sttk sttk commented May 27, 2017

I implemented for the issue #110.

Since Appveyor failed, I modified appveyor.yml, too.

@sttk
Copy link
Contributor Author

sttk commented May 31, 2017

I've modified according to #114

Copy link
Member

@phated phated left a comment

Choose a reason for hiding this comment

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

Nice and simple. 1 question about the appveyor changes.

@@ -15,8 +10,6 @@ environment:
- nodejs_version: "6"

install:
- npm -g install npm@latest
Copy link
Member

Choose a reason for hiding this comment

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

Are we going to run into weird test runs on old node versions with this removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@phated The reason of removing this line is that the latest npm don’t support old versions of nodejs (< v4.x). This is needed if we are going to support gulp-cli on these old versions and use Appveyor for test on Windows.

Or we might be able to skip test on Windows about old versions of nodejs.

Copy link
Member

Choose a reason for hiding this comment

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

Not dropping 0.10.

Just install npm at a good version, like 2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@phated If the “weird test” means using gulp-test-tool, since this tool is a thing to solve this issue on nodejs v0.10 on Windows, we can remove this tool if we skip test on Windows about old versions of nodejs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My reply crossed with yours.

Just install npm at a good version, like 2

I got it. I checked our test succeeded with npm@2 now.

@phated phated merged commit 80c0510 into gulpjs:master Jun 7, 2017
@sttk
Copy link
Contributor Author

sttk commented Jun 7, 2017

@phated Thanks for merging.

@sttk sttk deleted the config_loglevel branch June 7, 2017 20:36
phated pushed a commit that referenced this pull request Dec 21, 2017
phated pushed a commit that referenced this pull request Dec 21, 2017
phated pushed a commit that referenced this pull request Dec 21, 2017
phated pushed a commit that referenced this pull request Dec 21, 2017
phated pushed a commit that referenced this pull request Dec 21, 2017
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