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(schematics): Use prefix option in feature schematic #3139

Merged
merged 5 commits into from
Sep 23, 2021
Merged

fix(schematics): Use prefix option in feature schematic #3139

merged 5 commits into from
Sep 23, 2021

Conversation

hemangsk
Copy link
Contributor

@hemangsk hemangsk commented Sep 5, 2021 •
edited
Loading

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Closes #3116

What is the new behavior?

When using the feature schematic with --prefix option, effects and reducers will use the supplied prefix. action already uses the supplied prefix.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@hemangsk
refactor(schematics): Move prefix logic to ngrx-utils
1ca0968 
This allows sharing the prefix logic with rest of the
schematics.
@hemangsk
feat(schematics): Add prefix support for effect
0193f87 
This allows generation of effects with a prefix.
@hemangsk hemangsk changed the title Hmg/schematics patch 1 fix(schematics): Use prefix option in feature schematic Sep 5, 2021
@hemangsk
feat(schematics): Add support for prefix in reducer schematic
8ce15b0 
This allows generating reducers with a prefix
@hemangsk
fix(schematics): Use prefix option in feature schematic
f1e03c7 
Previously, passing --prefix when generating a feature would
make changes only in the generated action. This passes the prefix
option to the effect and reducers as well.

Fixes ngrx#3116
@hemangsk hemangsk force-pushed the hmg/schematics-patch-1 branch from 1ddec0f to f1e03c7 Compare September 5, 2021 09:47
@timdeschryver
Copy link
Member

Hi @hemangsk thanks for opening a PR.
Do you think that this is ready to be reviewed?

@hemangsk hemangsk marked this pull request as ready for review September 20, 2021 10:17
@hemangsk
Copy link
Contributor Author

Hi @timdeschryver, yes this is ready for review now, can you please take a look?

@timdeschryver
Copy link
Member

Thanks @hemangsk , we'll take a look as soon as we can.

timdeschryver
timdeschryver reviewed Sep 21, 2021
View reviewed changes
modules/schematics/src/effect/index.spec.ts
@@ -463,4 +464,35 @@ describe('Effect Schematic', () => {

expect(content).toMatch(/effects = TestBed\.inject\(FooEffects\);/);
});

it('should add prefix to the effect', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

One small remark, could you also write a test for when the action creators option is true please, because that's the one that is most used. The rest is looking good 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@timdeschryver , added a test with creators option set as true for in the effect spec file. I'm not sure but do we need tests with creators: true in feature and reducer as well?

Copy link
Member

@timdeschryver timdeschryver Sep 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The feature and reducer tests are by default creators: true, so this is fine for me.
Thanks for updating the PR! 👍

@hemangsk hemangsk force-pushed the hmg/schematics-patch-1 branch from 7b42544 to 3ec9173 Compare September 22, 2021 11:52
@timdeschryver timdeschryver merged commit 5fa8890 into ngrx:master Sep 23, 2021
@timdeschryver
Copy link
Member

Thanks @hemangsk !

@hemangsk hemangsk deleted the hmg/schematics-patch-1 branch September 24, 2021 06:51
This was referenced Nov 1, 2021
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timdeschryver timdeschryver timdeschryver approved these changes

@brandonroberts brandonroberts brandonroberts approved these changes

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

Successfully merging this pull request may close these issues.

[schematics/feature] Prefix option is not used when generating effects and reducers
3 participants
@hemangsk @timdeschryver @brandonroberts