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.

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

This closes #674, closes #1454, add new exported functions GetTables and DeleteTable #1573

Merged
merged 11 commits into from
Aug 23, 2023
Merged

This closes #674, closes #1454, add new exported functions GetTables and DeleteTable #1573

merged 11 commits into from
Aug 23, 2023

Conversation

fsfsx
Copy link
Contributor

@fsfsx fsfsx commented Jul 9, 2023

PR Details

Create 2 functions GetTables and DeleteTable to support reading and removing tables (working in progress).

Description

Support to get the table name, table range and the table style name
Implement this feature in the table.go source code

Related Issue

#1454, #674

Motivation and Context

How Has This Been Tested

Complete documentation and unit tests, incremental test coverage was 100% (working in progress)

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@fsfsx fsfsx changed the title WIP - This closes #1454, add new exported function GetTables Draft - This closes #1454, add new exported function GetTables Jul 9, 2023
@fsfsx fsfsx changed the title Draft - This closes #1454, add new exported function GetTables Draft This closes #1454, add new exported function GetTables Jul 9, 2023
@fsfsx fsfsx marked this pull request as draft July 9, 2023 13:20
@fsfsx fsfsx changed the title Draft This closes #1454, add new exported function GetTables This closes #1454, add new exported function GetTables Jul 9, 2023
taomin597715379
taomin597715379 requested changes Jul 10, 2023
View reviewed changes
table.go Show resolved Hide resolved
table.go Show resolved Hide resolved
@fsfsx fsfsx requested a review from taomin597715379 July 19, 2023 15:00
taomin597715379
taomin597715379 requested changes Jul 20, 2023
View reviewed changes
Copy link
Collaborator

@taomin597715379 taomin597715379 left a comment

Choose a reason for hiding this comment

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

Good job. Please add the unit test for the GetTables function.

taomin597715379
taomin597715379 requested changes Aug 19, 2023
View reviewed changes
Copy link
Collaborator

@taomin597715379 taomin597715379 left a comment

Choose a reason for hiding this comment

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

The GetTables functions implementation looks good to me, I suggest that add the DeleteTable function in this change list.

@fsfsx fsfsx changed the title This closes #1454, add new exported function GetTables This closes #1454, add new exported functions GetTables and DeleteTable Aug 21, 2023
@fsfsx fsfsx changed the title This closes #1454, add new exported functions GetTables and DeleteTable This closes #674, closes #1454, add new exported functions GetTables and DeleteTable Aug 21, 2023
@fsfsx fsfsx requested a review from taomin597715379 August 21, 2023 04:15
@fsfsx
Copy link
Contributor Author

fsfsx commented Aug 21, 2023

Done. I have added the DeleteTable function. Please take a look.

taomin597715379
taomin597715379 requested changes Aug 21, 2023
View reviewed changes
table.go Outdated Show resolved Hide resolved
@fsfsx fsfsx marked this pull request as ready for review August 21, 2023 16:29
@fsfsx fsfsx requested a review from taomin597715379 August 21, 2023 16:29
taomin597715379
taomin597715379 requested changes Aug 22, 2023
View reviewed changes
table_test.go Outdated Show resolved Hide resolved
@fsfsx fsfsx requested a review from taomin597715379 August 22, 2023 16:33
taomin597715379
taomin597715379 approved these changes Aug 23, 2023
View reviewed changes
Copy link
Collaborator

@taomin597715379 taomin597715379 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for your contribution.

@taomin597715379 taomin597715379 added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 23, 2023
@taomin597715379 taomin597715379 merged commit cb5a8e2 into qax-os:master Aug 23, 2023
jenbonzhang pushed a commit to jenbonzhang/excelize that referenced this pull request Oct 22, 2023
@fsfsx
This closes qax-os#674, closes qax-os#1454, add new exported function…
bdbb39e 
…s GetTables and DeleteTable (qax-os#1573)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@taomin597715379 taomin597715379 taomin597715379 approved these changes

Assignees
No one assigned
Labels
size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fsfsx @taomin597715379