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

Adding web-app flag #395

Merged
merged 5 commits into from
Jan 13, 2020
Merged

Adding web-app flag #395

merged 5 commits into from
Jan 13, 2020

Conversation

aalykiot
Copy link
Member

@aalykiot aalykiot commented Jan 9, 2020

I'm re-opening the PR from the new branch 'web-app-flag'. Still WIP but the linting error is fixed and I also updated the docs, so the only thing left is writing the tests :)

@coveralls
Copy link

coveralls commented Jan 9, 2020

Pull Request Test Coverage Report for Build 1050

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.01%) to 96.659%

Totals Coverage Status
Change from base Build 614: 0.01%
Covered Lines: 637
Relevant Lines: 662

💛 - Coveralls

@lholmquist lholmquist self-requested a review January 9, 2020 15:36
@lholmquist
Copy link
Member

looking pretty good, and for the test, you should be able to copy something from https://github.com/nodeshift/nodeshift/blob/master/test/build-strategy-test.js or https://github.com/nodeshift/nodeshift/blob/master/test/definitions-tests/build-strategy-test.js which just realized should be merged together. I'll create another issue for that

@aalykiot
Copy link
Member Author

@lholmquist I finished writing the tests (It's just a single test though). I think the PR is ready for a final review and merge :)

@aalykiot aalykiot marked this pull request as ready for review January 10, 2020 13:48
@aalykiot aalykiot changed the title WIP: Adding web-app flag Adding web-app flag Jan 10, 2020
Copy link
Member

@lholmquist lholmquist left a comment

Choose a reason for hiding this comment

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

👍

@lholmquist lholmquist added this to In progress in Tooling and Modules Jan 13, 2020
@lholmquist lholmquist moved this from In progress to Review in progress in Tooling and Modules Jan 13, 2020
@lholmquist
Copy link
Member

@alexalikiotis since we merged #397, the test file you added your tests to doesn't exist anymore :) can you rebase this PR with master.

@aalykiot
Copy link
Member Author

@lholmquist Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants