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

Scorpiocoding #101

Closed
wants to merge 5 commits into from
Closed

Scorpiocoding #101

wants to merge 5 commits into from

Conversation

ScorpioCoding
Copy link

Hi there,

Created a hotfix for the gulp-util deprecation issue
Installed dependecies

  • plugin-error as replacement for gulp-util.PluginError
  • fancy-log as replacement for gulp-util.log

files modified are:
.gitignore
--- added package-lock.json that occures when using VS code and SourceTree Git manager
package.json
--- added dependencies
index.js
--- mod code

I hope that my first try is a succes?
my firdst pull-request....lol

kind regards
scorpiocoding - kribo

@OverZealous
Copy link
Owner

This is a duplicate of #100, but thank you for the PR.

@OverZealous OverZealous closed this Jan 3, 2018
@demurgos
Copy link
Contributor

demurgos commented Jan 3, 2018

@ScorpioCoding
Since it's one of your first PRs, here are some tips:

  • Before starting a fix, check issues and PRs: a fix may already be there. You should also check closed issues/PRs because the fix may already be merged and just waiting to be published. This allows to avoid duplicated effort.
  • Use a descriptive title for your PR (ie. "what it does"). We can see your username pretty fine in the PR, no need to add it in the title.
  • Try to avoid to do too many commits (especially for small changes like this one): they obfuscate the change history. For larger PRs that need to be broken down into multiple commits, you'd like to keep each individual commit to work. It's the case there, but it often does not work to do one commit per edited file.
  • Regarding the PR itself, remove unused code instead of commenting it. Git is there to track the history of the files, no need to keep old code around (it's just confusing: why is it commented? do we want to use it in the future?). If the old code is really important, a comment with the git hash is enough. You should also avoid the sc mod noise
  • Finally, make sure to solve the issue 😄. I guess that it's a mistake but you forgot to actually remove gulp-util from the dependencies (package.json).

Thanks again for your help. We hope you continue to help projects you use, experience will come with practice.

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.

None yet

4 participants