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

[Enhance] enhance fetch worksheet by title parsing, improve code readability #6977

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mdirfan40
Copy link

@mdirfan40 mdirfan40 commented May 17, 2024

What type of PR is this?

i wan to Fix #6409

  • don't use mixed-type variables
  • allow for non-quoted worksheet titles (single word)
  • verify 0/1 indexing of worksheet numbers, and add comment if it differs
  • Refactor
  • Feature
  • Bug Fix
  • New Query Runner (Data Source)
  • New Alert Destination
  • Other

How is this tested?

  • Unit tests (pytest, jest)
  • E2E Tests (Cypress)
  • Manually
  • N/A

@mdirfan40
Copy link
Author

Getting this error while back lint check how to fix it ?

Oh no! 💥 💔 💥
1 file would be reformatted, 252 files would be left unchanged.

@justinclift
Copy link
Member

Getting this error while back lint check how to fix it ?

Did you set up your development environment using this wiki page?

https://github.com/getredash/redash/wiki/Local-development-setup

There's a section at the bottom of that which talks about the "Configuring "Pre-commit":

https://github.com/getredash/redash/wiki/Local-development-setup#configuring-pre-commit

I think that's the piece which takes care of formatting issues (the "lint" check things).

@mdirfan40
Copy link
Author

Any feedback and changes required to merge this PR ?

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.

Google Spreadsheets worksheet by title cleanup/refactor
2 participants