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.

Sign up for GitHub

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

Jump to bottom

fix: cypress-schematic add exception for nguniversal ssr dev server #23348

Merged
merged 10 commits into from
Aug 17, 2022
Merged

fix: cypress-schematic add exception for nguniversal ssr dev server #23348

merged 10 commits into from
Aug 17, 2022

Conversation

conradkirschner
Copy link
Contributor

User facing changelog

Allow optional watch option for angular universal dev server

How has the user experience changed?

It will now work with angular universal dev server

PR Tasks

  • [ na] Have tests been added/updated?
  • Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
  • [na] Has a PR for user-facing changes been opened in cypress-documentation?
  • [na] Have API changes been updated in the type definitions?

@conradkirschner conradkirschner requested a review from a team as a code owner August 15, 2022 14:17
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Aug 15, 2022

Thanks for taking the time to open a PR!

@CLAassistant
Copy link

CLAassistant commented Aug 15, 2022 •
edited

CLA assistant check
All committers have signed the CLA.

@conradkirschner conradkirschner changed the title fix: cypress-schematic remove watch option if dev server dont support it fix: cypress-schematic add exception for nguniversal ssr dev server Aug 15, 2022
@ZachJW34 ZachJW34 requested a review from admah August 15, 2022 22:38
lmiller1990
lmiller1990 approved these changes Aug 16, 2022
View reviewed changes
Copy link
Contributor

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

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

Just a nit, not blocking - will approve so once the final update is made we can merge.

npm/cypress-schematic/src/builders/cypress/index.ts Outdated Show resolved Hide resolved
ZachJW34
ZachJW34 reviewed Aug 16, 2022
View reviewed changes
Copy link
Contributor

@ZachJW34 ZachJW34 left a comment

Choose a reason for hiding this comment

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

One last tweak then should be good to go!

npm/cypress-schematic/src/builders/cypress/index.ts Outdated Show resolved Hide resolved
npm/cypress-schematic/src/builders/cypress/index.ts Outdated Show resolved Hide resolved
ZachJW34
ZachJW34 approved these changes Aug 16, 2022
View reviewed changes
Copy link
Contributor

@ZachJW34 ZachJW34 left a comment

Choose a reason for hiding this comment

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

LGTM!!!

@ZachJW34 ZachJW34 changed the base branch from develop to master August 16, 2022 22:18
@ZachJW34
Copy link
Contributor

ZachJW34 commented Aug 16, 2022 •
edited

@conradkirschner I changed the destination branch to master as that is our branch that will automatically release our npm packages come merge

@lmiller1990 lmiller1990 merged commit 1f05ff0 into cypress-io:master Aug 17, 2022
@conradkirschner conradkirschner deleted the cypress-schematic/optional-watch-23345 branch August 17, 2022 05:27
@conradkirschner
Copy link
Contributor Author

@ZachJW34 It got merged but not published. [workflow](https://github.com/cypress-io/cypress/actions/runs/2871983599
Looks like bot doesnt have permission for merge into dev and it wasnt published on npm.

@ZachJW34 @lmiller1990

@ZachJW34
Copy link
Contributor

@conradkirschner that failure is (unfortunately) expected, but there were some tests that failed which look like they don't pertain to your changes. I'm rerunning CI, the npm-release job is the job to keep an eye on

@ZachJW34
Copy link
Contributor

@cypress/schematic@2.0.3 has been published!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lmiller1990 lmiller1990 lmiller1990 approved these changes

@ZachJW34 ZachJW34 ZachJW34 approved these changes

@admah admah Awaiting requested review from admah

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

@cypress/schematics Universal Dev Server - No Watch Option
4 participants
@conradkirschner @CLAassistant @ZachJW34 @lmiller1990