Skip to content

Remove lodash dependency #37

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 3 commits into from
Oct 14, 2021
Merged

Remove lodash dependency #37

merged 3 commits into from
Oct 14, 2021

Conversation

@jorenvandeweyer
Copy link
Member

@jorenvandeweyer jorenvandeweyer commented Oct 13, 2021

Resolves issue #23

@HappyZombies
Copy link
Member

HappyZombies commented Oct 13, 2021

Awesome, we will take a look at this soon after #30 is merged into development. But so far looks good, excited for this removal.

@HappyZombies HappyZombies added dependencies 🔌 Pull requests that update a dependency file on hold 🛑 We will look into it at a later time labels Oct 13, 2021
@jwerre
Copy link
Contributor

jwerre commented Oct 13, 2021

Hey! I was just getting ready to do this. Thanks @jorenvandeweyer I'll check it out now.

@HappyZombies HappyZombies self-requested a review October 13, 2021 14:33
jwerre
jwerre reviewed Oct 13, 2021
View reviewed changes
jwerre
jwerre reviewed Oct 13, 2021
View reviewed changes
jwerre
jwerre reviewed Oct 13, 2021
View reviewed changes
@jankapunkt jankapunkt linked an issue Oct 13, 2021 that may be closed by this pull request
Remove lodash dependency #23
Closed
jankapunkt
jankapunkt reviewed Oct 13, 2021
View reviewed changes
Copy link
Member

@jankapunkt jankapunkt 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 to me, I have only the one thing to discuss but won't block due to it. I'd rather want to hear what you think. We can also create a separate issue on this one.

@jankapunkt
Copy link
Member

jankapunkt commented Oct 13, 2021

@jorenvandeweyer can you please pull development? We merged a few PRs lately

jankapunkt
jankapunkt approved these changes Oct 14, 2021
View reviewed changes
Copy link
Member

@jankapunkt jankapunkt 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. I would squash merge this into one commit and name it using our commit conventions. If no one has any arguments against I would proceed.

@HappyZombies
Copy link
Member

HappyZombies commented Oct 14, 2021 •
edited
Loading

@jankapunkt I haven't had much time to look at myself but I trust you, you are good to merge!

@HappyZombies HappyZombies removed the on hold 🛑 We will look into it at a later time label Oct 14, 2021
HappyZombies
HappyZombies approved these changes Oct 14, 2021
View reviewed changes
Copy link
Member

@HappyZombies HappyZombies left a comment

Choose a reason for hiding this comment

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

Excellent!

@HappyZombies HappyZombies merged commit 8e05cb5 into node-oauth:development Oct 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jankapunkt jankapunkt jankapunkt approved these changes

@HappyZombies HappyZombies HappyZombies approved these changes

+1 more reviewer

@jwerre jwerre jwerre approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

dependencies 🔌 Pull requests that update a dependency file

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove lodash dependency

4 participants

@jorenvandeweyer @HappyZombies @jwerre @jankapunkt