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

Issue #687 : Add feature to set position of worksheet #688

Merged
merged 1 commit into from
Mar 23, 2020

Conversation

djmgit
Copy link
Contributor

@djmgit djmgit commented Jun 30, 2019

Solves issue #687. Added param index in add_worksheet method
to set index of the newly created worksheet

@djmgit
Copy link
Contributor Author

djmgit commented Jun 30, 2019

@burnash please have a look.

@lucasklaassen
Copy link

This is a good change to make. Would be interested in seeing this be merged!

@@ -269,6 +271,9 @@ def add_worksheet(self, title, rows, cols):
}]
}

if index:
Copy link

@lucasklaassen lucasklaassen Nov 17, 2019

Choose a reason for hiding this comment

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

Suggested change
if index:
if index is not None:

If index is set to 0, this will return false and not send the index to google. Setting an index of 0 is very useful when you want your current worksheet to be the first worksheet within the spreadsheet.

@djmgit
Copy link
Contributor Author

djmgit commented Nov 18, 2019

@lucasklaassen thanks for the review, I will update my PR as soon as possible

Solves issue burnash#687. Added param index in add_worksheet method
to set index of the newly created worksheet
@djmgit
Copy link
Contributor Author

djmgit commented Dec 14, 2019

@lucasklaassen done with the change you suggested.

Copy link

@lucasklaassen lucasklaassen left a comment

Choose a reason for hiding this comment

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

LGTM

@lucasklaassen
Copy link

@burnash this change is valuable and the code looks good. Can you please review and merge?

@burnash burnash merged commit 4893bed into burnash:master Mar 23, 2020
@burnash
Copy link
Owner

burnash commented Mar 23, 2020

@djmgit thank you for your contribution.
@lucasklaassen thank you for the review.
Sorry to everyone for the late reply.

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