Skip to content

Support Short-circuit load#261

Merged
unknwon merged 2 commits intogo-ini:masterfrom
ygj6:issue180
Sep 5, 2020
Merged

Support Short-circuit load#261
unknwon merged 2 commits intogo-ini:masterfrom
ygj6:issue180

Conversation

@ygj6
Copy link
Copy Markdown
Contributor

@ygj6 ygj6 commented Sep 1, 2020

What problem should be fixed?

Short-circuit load #180

  1. The ShortCircuit option is adde
  2. A judgment is added to the Reload() method to implement short-circuit load.

Note: When ShortCircuit and Loose are both true, ShortCircuit has priority over Loose.

Have you added test cases to catch the problem?

Yes, all CI pass the test.

Copy link
Copy Markdown
Member

@unknwon unknwon left a comment

Choose a reason for hiding this comment

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

Thanks! Please see inline comments.

Comment thread file.go Outdated
Comment thread file.go Outdated
Comment thread ini.go Outdated
@unknwon unknwon added the status: reviewed The pull request has been reviewed label Sep 3, 2020
Copy link
Copy Markdown
Member

@unknwon unknwon 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 the follow up! Few more styling comments.

Comment thread file.go
Comment thread ini.go Outdated
Comment thread file.go Outdated
Comment thread file.go Outdated
Copy link
Copy Markdown
Member

@unknwon unknwon left a comment

Choose a reason for hiding this comment

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

Thank you for keeping this up!

Comment thread ini_test.go Outdated
Comment thread ini_test.go Outdated
@unknwon unknwon merged commit fcd0515 into go-ini:master Sep 5, 2020
@unknwon unknwon mentioned this pull request Sep 5, 2020
@unknwon
Copy link
Copy Markdown
Member

unknwon commented Sep 5, 2020

https://github.com/go-ini/ini/releases/tag/v1.61.0 has tagged for this merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: reviewed The pull request has been reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants