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.

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

Add @types/jest, upgrade example app dependencies, README and CHANGELOG #116

Merged
merged 3 commits into from
Jan 27, 2018
Merged

Add @types/jest, upgrade example app dependencies, README and CHANGELOG #116

merged 3 commits into from
Jan 27, 2018

Conversation

ahnpnl
Copy link
Collaborator

@ahnpnl ahnpnl commented Jan 27, 2018

No description provided.

Copy link
Owner

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

Looks good, left some thoughts inlined

CHANGELOG.md Outdated
@@ -4,6 +4,9 @@
* Breaking: Upgrade Jest to 22 ([#109](https://github.com/thymikee/jest-preset-angular/pull/109))
* Breaking: Upgrade `ts-jest` to 22 ([#109](https://github.com/thymikee/jest-preset-angular/pull/109))
* Chore: Get rid of explicit `jsdom` dependency and custom test environment
* Fix: Upgrade example app to Angular 5.2.x using Angular CLI 1.6.6
* Fix: Add @types/jest as a package dependency
* Fix: Adjust README for easy installation guide and add configuration section with vendor libaries like jQuery etc..
Copy link
Owner

Choose a reason for hiding this comment

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

Can you move these to master change log and link to PT?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

do you mean I should commit these changes to CHANGELOG.md on master branch and mention these changes with PR behind ?

Copy link
Owner

Choose a reason for hiding this comment

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

No, by master I mean master changelog. The first line of this file states:

## Changelog (master)

Since these "fixes" are not part of v5.0.0 release, let's just move them up:

## Changelog (master)

* Feature: Simplify installation by adding @types/jest as a package dependency ([#116](...))
* Chore: Upgrade example app to Angular 5.2 using Angular CLI 1.6 ([#116](...))

### v5.0.0
**v5.0.0 stuff, do not touch**

And later with a followup PR:

* Docs: Add a configuration section with vendor libraries like jQuery ([#117](...))

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah ok I got it

README.md Outdated
@@ -222,3 +224,22 @@ import 'rxjs/add/operator/catch';

import './jestGlobalMocks';
```

### Allow vendor libraries like jQuery, etc...
Copy link
Owner

Choose a reason for hiding this comment

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

Can we separate this into its own PR? I'd like not to mix unrelated changes in one shot

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok I think that's better

@thymikee thymikee merged commit 2273e1f into thymikee:master Jan 27, 2018
@thymikee
Copy link
Owner

Thanks!

@ahnpnl ahnpnl deleted the fix/add-type-jest branch January 27, 2018 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants