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

Updating dependencies #44

Merged
merged 5 commits into from
Feb 15, 2019
Merged

Updating dependencies #44

merged 5 commits into from
Feb 15, 2019

Conversation

MarhyCZ
Copy link
Contributor

@MarhyCZ MarhyCZ commented Sep 26, 2018

Hi, nothing exciting with new features, but I updated bundled lodash and pubsub-js, so users dont have to load another lodash if they need it and the atvjs package wont show any vulnerabilites in github.

Bundled dependencies "lodash" and "pubsub-js" were updated.
Feature, that adds atribute to menu item, which allow to reload page on each
@emadalam
Copy link
Owner

Hey @MarhyCZ!

Thanks a ton for the commit, much appreciate. Just came back from a vacation, so apologies for the delay in response.

The commits look great, however I have few concerns:

  • lodash version update has a major version bump that means some breaking changes. atvjs uses lodash internally as well for some manipulations. Would you mind having a look at what are the breaking changes from v3 to v4 and if those are used anywhere in the atvjs code 🤓
  • Can you add a line or two in the README docs for reloadOnSelect 📚 💪
  • You can leave the version update on me, as it gets automatically updated when I push the package to npm repository 📦

Other than these, all looks great 👍 And thanks again for taking out time to make changes to the repo, I highly appreciate 🙏

@MarhyCZ
Copy link
Contributor Author

MarhyCZ commented Feb 15, 2019

Hi @emadalam ! :)

I am also very sorry for this delay, had a too much to do with school. Hope you enjoyed vacation!

  • I have gone through all lodash calls. None of the functions you are using were removed, renamed or changed. I am actually using this build with my apps and it works ok (navigation, loading, everything seems to work properly) And the 4.17.11 is still the newest version
  • I have added reloadOnSelect to README in the Navigation section

And I thank you for awesome, easy JS framework, that allows my to make simple content apps! :) So at least I can contribute with occasionally updating the deps.

Have a nice day 😊

@emadalam emadalam merged commit e42a538 into emadalam:master Feb 15, 2019
@emadalam
Copy link
Owner

Hey @MarhyCZ

Thanks a ton for this, highly appreciate, every contribution counts 💯Glad to know the library is of help, spread the love 😊

Good luck with your school 💪

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.

None yet

2 participants