Skip to content

fix rows next#503

Merged
xuri merged 1 commit intoqax-os:masterfrom
ducquangkstn:fix-rows-next
Oct 18, 2019
Merged

fix rows next#503
xuri merged 1 commit intoqax-os:masterfrom
ducquangkstn:fix-rows-next

Conversation

@ducquangkstn
Copy link
Copy Markdown
Contributor

@ducquangkstn ducquangkstn commented Oct 18, 2019

PR Details

Fix bug

Description

rows.Next should increase the curRow variable
while rows.Columns shouldn't do it

Related Issue

#502

Motivation and Context

How Has This Been Tested

  • Add a unit test. Using require to cancel the test immediately instead of
if !assert.NoError(t, rows.Error()) {
	t.FailNow()
}

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.

@ducquangkstn ducquangkstn marked this pull request as ready for review October 18, 2019 04:40
Copy link
Copy Markdown
Member

@xuri xuri 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 your PR. I left some comments.

Comment thread rows.go Outdated
Comment thread rows_test.go Outdated
Comment thread rows.go Outdated
@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 18, 2019

Codecov Report

Merging #503 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #503   +/-   ##
=======================================
  Coverage   96.26%   96.26%           
=======================================
  Files          26       26           
  Lines        5513     5513           
=======================================
  Hits         5307     5307           
  Misses        115      115           
  Partials       91       91
Impacted Files Coverage Δ
rows.go 84.86% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e791fa...badaa22. Read the comment docs.

fix rows next

fix rows next

fix rows next

fix rows next
Comment thread rows.go Outdated
@ducquangkstn
Copy link
Copy Markdown
Contributor Author

Change as requested

@xuri xuri merged commit 866fda2 into qax-os:master Oct 18, 2019
@mlh758
Copy link
Copy Markdown
Contributor

mlh758 commented Oct 23, 2019

Sorry about that!

@xuri xuri added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jan 21, 2020
nullfy pushed a commit to nullfy/excelize that referenced this pull request Oct 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants