Skip to content

Conversation

benatespina
Copy link
Member

@benatespina benatespina commented Dec 28, 2018

  • Upgraded dependencies.

@benatespina benatespina force-pushed the feature/upgrade-dependencies branch from 92cd2fa to 59bc687 Compare December 28, 2018 17:36
CHANGELOG.md Outdated
- [BC BREAK] Removed support for React and ReactDOM < 16.3 (Context API is not supported in those versions)
- Added React component testing using `react-testing-library`
- Increased the test code coverage
- Upgraded all dependencies removing alpha and beta versions
Copy link
Contributor

Choose a reason for hiding this comment

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

v0.8.0 was already published.

This should go in v0.8.1

Copy link
Member Author

Choose a reason for hiding this comment

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

mmmm ok, I didn't know that the v0.8.0 was already published. So, first of all we should tag this version in GitHub's releases

Copy link
Member Author

Choose a reason for hiding this comment

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

Moreover, we can merge this PR without publishing any new release. It's a bug-fix...

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it's a bugfix. Therefore it should be 0.8.1 instead of 0.9.0

Copy link
Contributor

Choose a reason for hiding this comment

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

mmmm ok, I didn't know that the v0.8.0 was already published. So, first of all we should tag this version in GitHub's releases

Yes, I forgot to tag it in GitHub 😓

Copy link
Member Author

Choose a reason for hiding this comment

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

jajaja sorry I'd want to say that it's NOT a bugfix, in the end you can work properly with master current dependencies.

Copy link
Member Author

Choose a reason for hiding this comment

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

pathFromRouteForPathsAndLocale,
),
renderTranslatedRoutesForCurrentConfig = renderTranslatedRoutesForConfig(
configRoute,
Copy link
Contributor

Choose a reason for hiding this comment

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

I would move this changes to another PR.

Changes look great but I would add tests for renderTranslatedRoutes to ensure we are not breaking anything

@benatespina
Copy link
Member Author

@gorkalaucirica ping

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