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

Remove unused variable assignment from Admin::OrdersController#index action #3170

Merged

Conversation

aitbw
Copy link
Contributor

@aitbw aitbw commented Apr 10, 2019

Description

This variable assignment was introduced back in commit 1a38331; since it is now unused and does not affect the code in any way, it can be safely removed

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)

@aitbw aitbw force-pushed the aitbw/remove-unnecessary-deletion branch from 35854d3 to f8d2dd2 Compare April 10, 2019 17:03
@aitbw aitbw changed the title Remove unnecessary deletion of date params when filtering orders Remove dead code when filtering orders Apr 10, 2019
@kennyadsl
Copy link
Member

I think params here are restored so they can be presented to the user in the view again. If they are left empty user would lose the date that he/she entered before submitting, right?

@aitbw
Copy link
Contributor Author

aitbw commented Apr 15, 2019

@kennyadsl oh snap, you're absolutely right! Sorry about that 🙇‍♀️

There's still this, that I think we can safely remove: query_present = params[:q]

This variable assignment was introduced back in commit 1a38331; since
it is now unused and does not affect the code in any way, it can be
safely removed
@aitbw aitbw force-pushed the aitbw/remove-unnecessary-deletion branch from f8d2dd2 to 442fe04 Compare April 15, 2019 19:52
@aitbw aitbw changed the title Remove dead code when filtering orders Remove unused variable assignment from Admin::OrdersController#index action Apr 15, 2019
Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

👍 Thanks Angel!

Copy link
Contributor

@jacobherrington jacobherrington left a comment

Choose a reason for hiding this comment

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

Thanks for catching this!

@kennyadsl kennyadsl merged commit f5b124c into solidusio:master May 2, 2019
@aitbw aitbw deleted the aitbw/remove-unnecessary-deletion branch May 3, 2019 17:19
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.

3 participants