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

Replace gulp-webserver with gulp-connect #215

Closed
wants to merge 9 commits into from
Closed

Replace gulp-webserver with gulp-connect #215

wants to merge 9 commits into from

Conversation

djasnowski
Copy link

This adheres issue #195 and successfully replaces said NPM package (which had 2 bad vulns). When I ran npm audit, it had no more security concerns.

~/code/oss/desktopJS git:(master) npm audit

                       === npm audit security report ===

found 0 vulnerabilities
 in 45568 scanned packages

Where can I find the Contributor License Agreement? Would I just talk to the email ([email protected]) asking for it?

@djasnowski djasnowski requested a review from a team February 7, 2019 18:38
@codecov
Copy link

codecov bot commented Feb 7, 2019

Codecov Report

Merging #215 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #215   +/-   ##
=======================================
  Coverage   93.44%   93.44%           
=======================================
  Files          16       16           
  Lines        1479     1479           
  Branches      249      249           
=======================================
  Hits         1382     1382           
  Misses         97       97

@bingenito
Copy link
Member

Thanks @naknode , I'll reply back on your mail about the CLA.

@@ -63,7 +63,7 @@ To run the examples or to manually test scenarios in each container, there is a
a local server with live reload. This can be run directly via gulp or through npm.

```
$ npm run server
$ npm run start
Copy link
Member

Choose a reason for hiding this comment

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

Excellent. I changed/broke that on the monorepo refactor

@djasnowski
Copy link
Author

Thanks @naknode , I'll reply back on your mail about the CLA.

Great! Can't wait to start contributing to Morgan Stanley and this project!

@djasnowski
Copy link
Author

DCO has been added and signed.

@bingenito
Copy link
Member

@naknode As it is required to indicate on each commit that it is covered by , I'll need updated commit messages. You can squash down to one commit if that is easier.

gulpfile.js Outdated Show resolved Hide resolved
@djasnowski
Copy link
Author

Would you mind showing me how to squash those commits? I'm trying to rebase and I thought I could do, but git rebase is proving to be more tricky than I thought. #220 (comment) is where I failed, too. Thank you.

@bingenito
Copy link
Member

@djasnowski
Copy link
Author

@bingenito Believe me, I tried. And tried. Git is a fickle thing. Maybe you can see #220 and see if that is applicable to be merged?

@bingenito
Copy link
Member

I need the dco on your side on each commit message to cover the submission. I think my merge of my PR in the middle is making this harder. When all else fails I usually just create a new PR.

@djasnowski
Copy link
Author

Closing.

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