Skip to content

refactor(example): remove @ngrx/db #1664

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
Merged
merged 1 commit into from
Apr 1, 2019
Merged

refactor(example): remove @ngrx/db #1664

merged 1 commit into from
Apr 1, 2019

Conversation

@King-i
Copy link
Contributor

@King-i King-i commented Mar 29, 2019

add localStorage replacement

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[x] 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 #1481

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@ngrxbot
Copy link
Collaborator

ngrxbot commented Mar 29, 2019

Preview docs changes for 9f93cbd at https://previews.ngrx.io/pr1664-9f93cbd/

timdeschryver
timdeschryver reviewed Mar 30, 2019
View reviewed changes
Copy link
Member

@timdeschryver timdeschryver left a comment

Choose a reason for hiding this comment

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

I left some comments

projects/example-app/src/app/core/services/book-storage.service.spec.ts
});
fixture = TestBed.get(BookStorageService);
});
describe('supported', () => {
Copy link
Member

@timdeschryver timdeschryver Mar 30, 2019

Choose a reason for hiding this comment

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

Nit, add a line break between describes / its

Copy link
Contributor Author

@King-i King-i Mar 30, 2019

Choose a reason for hiding this comment

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

Formatting is default vscode i believe, it also matches all other spec formatting in the project?

Copy link
Member

@timdeschryver timdeschryver Apr 1, 2019

Choose a reason for hiding this comment

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

I'm not sure if VSCode does this 🤔

Copy link
Contributor Author

@King-i King-i Apr 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Apologies you mentioned between describes / its, which it must do as I use a vanilla install of vscode with default settings, unless its the prettier formatter? if it was between the beforeEach and describe only you intended then i have already actioned. I also believe the project runs a pre-commit formatting task.

@ngrxbot
Copy link
Collaborator

ngrxbot commented Mar 30, 2019

Preview docs changes for 5725781 at https://previews.ngrx.io/pr1664-5725781/

@ngrxbot
Copy link
Collaborator

ngrxbot commented Mar 30, 2019

Preview docs changes for 2112bf6 at https://previews.ngrx.io/pr1664-2112bf6/

@King-i King-i force-pushed the 1481-remove-db-pr branch from 924298b to 4283711 Compare March 30, 2019 16:54
@ngrxbot
Copy link
Collaborator

ngrxbot commented Mar 30, 2019

Preview docs changes for 924298b at https://previews.ngrx.io/pr1664-924298b/

@ngrxbot
Copy link
Collaborator

ngrxbot commented Mar 30, 2019

Preview docs changes for 4283711 at https://previews.ngrx.io/pr1664-4283711/

timdeschryver
timdeschryver requested changes Apr 1, 2019
View reviewed changes
Copy link
Member

@timdeschryver timdeschryver left a comment

Choose a reason for hiding this comment

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

Also remove (?!db) inside example-app/jest.config.js

brandonroberts
brandonroberts requested changes Apr 1, 2019
View reviewed changes
@brandonroberts brandonroberts added the Needs Cleanup Review changes needed label Apr 1, 2019
@King-i King-i force-pushed the 1481-remove-db-pr branch from 4283711 to 7aea87d Compare April 1, 2019 13:27
brandonroberts
brandonroberts requested changes Apr 1, 2019
View reviewed changes
@ngrxbot
Copy link
Collaborator

ngrxbot commented Apr 1, 2019

Preview docs changes for 7aea87d at https://previews.ngrx.io/pr1664-7aea87d/

@King-i King-i force-pushed the 1481-remove-db-pr branch from 7aea87d to 7aee98d Compare April 1, 2019 13:42
@brandonroberts
Copy link
Member

brandonroberts commented Apr 1, 2019 •
edited
Loading

@King-i this needs a rebase on master to fix the conflicts from adding the createEffect feature

@King-i
refactor(example): remove @ngrx/db
b4be5be 
add localStorage replacement
@King-i King-i force-pushed the 1481-remove-db-pr branch from 7aee98d to b4be5be Compare April 1, 2019 13:51
@ngrxbot
Copy link
Collaborator

ngrxbot commented Apr 1, 2019

Preview docs changes for b4be5be at https://previews.ngrx.io/pr1664-b4be5be/

@brandonroberts
Copy link
Member

brandonroberts commented Apr 1, 2019

LGTM. Needs approval from @timdeschryver for his review comments.

timdeschryver
timdeschryver approved these changes Apr 1, 2019
View reviewed changes
Copy link
Member

@timdeschryver timdeschryver left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@timdeschryver timdeschryver merged commit f96974f into ngrx:master Apr 1, 2019
@King-i King-i deleted the 1481-remove-db-pr branch April 1, 2019 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brandonroberts brandonroberts brandonroberts approved these changes

@timdeschryver timdeschryver timdeschryver approved these changes

Assignees

No one assigned

Labels

Needs Cleanup Review changes needed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Example: Remove @ngrx/db

4 participants

@King-i @ngrxbot @brandonroberts @timdeschryver