-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
docs: update example app to standalone #4487
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for ngrx-io canceled.Built without sensitive environment variables
|
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.
Thanks for your work on this @sheikalthaf .
I left a few comments for some details.
Can you also take a look at the pipeline, it currently has some failing tests and ESLint issues.
For more details see https://github.com/ngrx/platform/pull/4487/checks?check_run_id=28492422455
45bb249
to
b741c7d
Compare
Hey, looks like we have a duplicate for this: #4417. Was there a second issue, or should I assign @sheikalthaf to it? |
@rainerhahnekamp it was assigned to you, so you still have first choice. |
@sheikalthaf, please keep in mind the contribution guide (https://github.com/ngrx/platform/blob/main/CONTRIBUTING.md). We usually open an issue and assign it to you. Only then should you submit a PR. Otherwise, multiple developers will be working on the same task. In this case, I am going to assign the issue to you, but please check the issues first for the next one. Please add a short comment in #4417, so that I can select you from the user list. Thanks! |
@rainerhahnekamp thanks for pointing out. sorry I don't know about that. From next time I'll check the issue comments clearly. Sorry for the confusion here. |
Woops, sorry @rainerhahnekamp I missed that during my initial review. |
@timdeschryver this is the only testcase is failing and i see that the snapshot is wrong. Can you help me here |
@rainerhahnekamp and me going to work together on this PR. For now I'm making this as a Draft until the pipeline is fixed. @timdeschryver FYI |
@sheikalthaf yup, I will check |
Updated the example app to standalone, removed all the modules from app.
d9321c3
to
d6cc1c7
Compare
Updated the example app to standalone, removed all the modules from app.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Closes #
What is the new behavior?
Does this PR introduce a breaking change?
Other information