This repository has been archived by the owner on Apr 9, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 156
fix(@schematic/angular): e2e should use the prefix #720
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cexbrayat
force-pushed
the
fix/e2e-prefix
branch
from
April 17, 2018 11:13
3d6f67f
to
7245ab5
Compare
cexbrayat
force-pushed
the
fix/e2e-prefix
branch
from
April 17, 2018 11:22
7245ab5
to
2edd648
Compare
hansl
reviewed
Apr 17, 2018
}, | ||
"prefix": { | ||
"description": "The prefix to apply.", | ||
"type": "string" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably have a default or some way to pickup the value from an existing project on e2e so people can do ng generate e2e
without a prefix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hansl I added a default (app
as we do everywhere for prefix. Also added format: html-selector
and alias: p
to be coherent.
Now that the CLI repects the prefix specified, the generated e2e test should also use it.
cexbrayat
force-pushed
the
fix/e2e-prefix
branch
from
April 17, 2018 17:14
2edd648
to
3717ecb
Compare
hansl
approved these changes
May 11, 2018
cexbrayat
added a commit
to cexbrayat/devkit
that referenced
this pull request
May 17, 2018
PR angular#906 updated the app schematics to display the name of the app in the title, so the e2e test has to be fixed. The prefix is now useless (it was introduced by angular#720 to fix the e2e test when it was using prefix), and has been removed.
cexbrayat
added a commit
to cexbrayat/devkit
that referenced
this pull request
May 17, 2018
PR angular#906 updated the app schematics to display the name of the app in the title, so the e2e test has to be fixed. The prefix is now useless (it was introduced by angular#720 to fix the e2e test when it was using prefix), and has been removed.
cexbrayat
added a commit
to cexbrayat/devkit
that referenced
this pull request
May 17, 2018
PR angular#906 updated the app schematics to display the name of the app in the title, so the e2e test has to be fixed. The prefix is now useless (it was introduced by angular#720 to fix the e2e test when it was using prefix), and has been removed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Now that the CLI repects the prefix specified, the generated e2e test should also use it.
Currently a project generated with rc.5 fails the e2e test right away.
Another solution is to avoid giving the prefix option to the e2e schematic, and simply revert the title field of
app.component.ts
toapp
. See #721