Update deps to fix infinite recursion in cyclic references#138
Merged
Update deps to fix infinite recursion in cyclic references#138
Conversation
Lines Of Code
|
Go API Changes# summary Inferred base version: v0.2.55 Suggested version: v0.2.56 |
Unit Test Coveragetotal: (statements) 45.7% Coverage of changed lines
Coverage diff with base branchNo changes in coverage. |
![llamapreview[bot]](https://avatars.githubusercontent.com/in/1023541?s=60&v=4){kind=small}
There was a problem hiding this comment.
Auto Pull Request Review from LlamaPReview
1. Overview
1.1 PR Summary
- Business value and requirements alignment: Fixes #137, which addresses an infinite recursion issue when handling self-referential structs.
- Key components modified:
openapi3/reflect_test.go,openapi31/reflect_test.go,operation.go,go.mod, andgo.sum. - Impact assessment: The fix updates the
refldependency to version1.3.1, which resolves the cyclic reference issue. This change might have implications for other packages or libraries that depend onreflor interact with theopenapi3andopenapi31packages. - System dependencies and integration impacts: The fix might have unintended side effects on other parts of the system, especially if there are other packages or libraries that depend on the
reflpackage or interact with theopenapi3andopenapi31packages. Thorough testing is required to ensure system stability.
2. Detailed Technical Analysis
2.1 Code Logic Deep-Dive
Core Logic Changes
- openapi3/reflect_test.go - TestSelfReference
- Submitted PR Code:
func TestSelfReference(t *testing.T) {
reflector := openapi3.NewReflector()
type SubEntity struct {
Self *SubEntity `json:"self"`
}
type My struct {
Foo string `json:"foo"`
SubEntity *SubEntity `json:"subentity"`
}
putOp, err := reflector.NewOperationContext(http.MethodPut, "/things/")
require.NoError(t, err)
putOp.AddReqStructure(My{})
putOp.AddRespStructure(My{})
require.NoError(t, reflector.AddOperation(putOp))
assertjson.EqMarshal(t, `{
"openapi":"3.0.3","info":{"title":"","version":""},
"paths":{
"/things/":{
"put":{
"requestBody":{
"content":{
"application/json":{"schema":{"$ref":"#/components/schemas/Openapi3TestMy"}}
}
},
"responses":{
"200":{
"description":"OK",
"content":{
"application/json":{"schema":{"$ref":"#/components/schemas/Openapi3TestMy"}}
}
}
}
}
}
},
"components":{
"schemas":{
"Openapi3TestMy":{
"type":"object",
"properties":{
"foo":{"type":"string"},
"subentity":{"$ref":"#/components/schemas/Openapi3TestSubEntity"}
}
},
"Openapi3TestSubEntity":{
"type":"object",
"properties":{"self":{"$ref":"#/components/schemas/Openapi3TestSubEntity"}}
}
}
}
}`, reflector.SpecSchema())
}- Analysis:
- The
TestSelfReferencefunction tests the handling of self-referential structs in OpenAPI 3.0.3. - It defines two structs,
SubEntityandMy, whereSubEntityhas a self-referential fieldSelf. - The function creates a new
Reflectorinstance, adds a PUT operation with request and response structures of typeMy, and then asserts that the generated OpenAPI schema matches the expected output. - The expected output includes the definition of the
Openapi3TestMyandOpenapi3TestSubEntityschemas, which correspond to theMyandSubEntitystructs, respectively.
- The
- LlamaPReview Suggested Improvements:
func TestSelfReference(t *testing.T) {
reflector := openapi3.NewReflector()
type SubEntity struct {
Self *SubEntity `json:"self"`
}
type My struct {
Foo string `json:"foo"`
SubEntity *SubEntity `json:"subentity"`
}
putOp, err := reflector.NewOperationContext(http.MethodPut, "/things/")
require.NoError(t, err)
putOp.AddReqStructure(My{})
putOp.AddRespStructure(My{})
require.NoError(t, reflector.AddOperation(putOp))
expectedSchema := `{
"openapi":"3.0.3","info":{"title":"","version":""},
"paths":{
"/things/":{
"put":{
"requestBody":{
"content":{
"application/json":{"schema":{"$ref":"#/components/schemas/Openapi3TestMy"}}
}
},
"responses":{
"200":{
"description":"OK",
"content":{
"application/json":{"schema":{"$ref":"#/components/schemas/Openapi3TestMy"}}
}
}
}
}
}
},
"components":{
"schemas":{
"Openapi3TestMy":{
"type":"object",
"properties":{
"foo":{"type":"string"},
"subentity":{"$ref":"#/components/schemas/Openapi3TestSubEntity"}
}
},
"Openapi3TestSubEntity":{
"type":"object",
"properties":{"self":{"$ref":"#/components/schemas/Openapi3TestSubEntity"}}
}
}
}
}`
assertjson.EqMarshal(t, expectedSchema, reflector.SpecSchema())
}- Improvement rationale:
- The suggested improvement adds a constant
expectedSchemato store the expected OpenAPI schema as a string. This change makes the test more maintainable, as the expected schema is now defined in a single place and can be easily updated if needed. - The improvement also removes the direct usage of
reflector.SpecSchema()in theassertjson.EqMarshalfunction, making the test more readable and easier to understand.
- The suggested improvement adds a constant
- Technical benefits:
- Improved maintainability: With the expected schema defined as a constant, it's easier to update the expected output if the schema changes in the future.
- Better readability: By removing the direct usage of
reflector.SpecSchema(), the test becomes more self-contained and easier to understand.
- Business value:
- More reliable tests: By having a constant for the expected schema, the test becomes more robust and less prone to errors due to changes in the schema.
- Risk assessment:
- The suggested improvement is a low-risk change, as it only involves refactoring the test to improve maintainability and readability. It doesn't change the behavior of the test or the system under test.
- openapi31/reflect_test.go - TestSelfReference
- Submitted PR Code:
func TestSelfReference(t *testing.T) {
reflector := openapi31.NewReflector()
type SubEntity struct {
Self *SubEntity `json:"self"`
}
type My struct {
Foo string `json:"foo"`
SubEntity *SubEntity `json:"subentity"`
}
putOp, err := reflector.NewOperationContext(http.MethodPut, "/things/")
require.NoError(t, err)
putOp.AddReqStructure(My{})
putOp.AddRespStructure(My{})
require.NoError(t, reflector.AddOperation(putOp))
assertjson.EqMarshal(t, `{
"openapi":"3.1.0","info":{"title":"","version":""},
"paths":{
"/things/":{
"put":{
"requestBody":{
"content":{
"application/json":{"schema":{"$ref":"#/components/schemas/Openapi31TestMy"}}
}
},
"responses":{
"200":{
"description":"OK",
"content":{
"application/json":{"schema":{"$ref":"#/components/schemas/Openapi31TestMy"}}
}
}
}
}
}
},
"components":{
"schemas":{
"Openapi31TestMy":{
"properties":{
"foo":{"type":"string"},
"subentity":{"$ref":"#/components/schemas/Openapi31TestSubEntity"}
},
"type":"object"
},
"Openapi31TestSubEntity":{
"properties":{"self":{"$ref":"#/components/schemas/Openapi31TestSubEntity"}},
"type":"object"
}
}
}
}`, reflector.SpecSchema())
}- Analysis:
- The
TestSelfReferencefunction inopenapi31/reflect_test.gois similar to the one inopenapi3/reflect_test.go, but it tests the handling of self-referential structs in OpenAPI 3.1.0. - The main difference is in the expected OpenAPI schema, which includes some minor changes to accommodate the differences between OpenAPI 3.0.3 and 3.1.0.
- The
- LlamaPReview Suggested Improvements:
func TestSelfReference(t *testing.T) {
reflector := openapi31.NewReflector()
type SubEntity struct {
Self *SubEntity `json:"self"`
}
type My struct {
Foo string `json:"foo"`
SubEntity *SubEntity `json:"subentity"`
}
putOp, err := reflector.NewOperationContext(http.MethodPut, "/things/")
require.NoError(t, err)
putOp.AddReqStructure(My{})
putOp.AddRespStructure(My{})
require.NoError(t, reflector.AddOperation(putOp))
expectedSchema := `{
"openapi":"3.1.0","info":{"title":"","version":""},
"paths":{
"/things/":{
"put":{
"requestBody":{
"content":{
"application/json":{"schema":{"$ref":"#/components/schemas/Openapi31TestMy"}}
}
},
"responses":{
"200":{
"description":"OK",
"content":{
"application/json":{"schema":{"$ref":"#/components/schemas/Openapi31TestMy"}}
}
}
}
}
}
},
"components":{
"schemas":{
"Openapi31TestMy":{
"properties":{
"foo":{"type":"string"},
"subentity":{"$ref":"#/components/schemas/Openapi31TestSubEntity"}
},
"type":"object"
},
"Openapi31TestSubEntity":{
"properties":{"self":{"$ref":"#/components/schemas/Openapi31TestSubEntity"}},
"type":"object"
}
}
}
}`
assertjson.EqMarshal(t, expectedSchema, reflector.SpecSchema())
}- Improvement rationale:
- The suggested improvement is the same as the one for
openapi3/reflect_test.go, adding a constantexpectedSchemato store the expected OpenAPI schema as a string and removing the direct usage ofreflector.SpecSchema()in theassertjson.EqMarshalfunction.
- The suggested improvement is the same as the one for
- Technical benefits:
- Improved maintainability and better readability, as explained in the previous improvement rationale.
- Business value:
- More reliable tests, as explained in the previous improvement rationale.
- Risk assessment:
- The suggested improvement is a low-risk change, as it only involves refactoring the test to improve maintainability and readability. It doesn't change the behavior of the test or the system under test.
- operation.go - WithOperationCtx
- Submitted PR Code:
type ocCtxKey struct{}
// WithOperationCtx is a jsonschema.ReflectContext option.
func WithOperationCtx(oc OperationContext, isProcessingResponse bool, in In) func(rc *jsonschema.ReflectContext) {
return func(rc *jsonschema.ReflectContext) {
oc.SetIsProcessingResponse(isProcessingResponse)
oc.SetProcessingIn(in)
rc.Context = context.WithValue(rc.Context, ocCtxKey{}, oc) //nolint:fatcontext
}
}
// OperationCtx retrieves operation context from reflect context.
func OperationCtx(rc *jsonschema.ReflectContext) (OperationContext, bool) {
if oc, ok := rc.Value(ocCtxKey{}).(OperationContext); ok {
return oc, true
}
return nil, false
}- Analysis:
- The
WithOperationCtxfunction is a jsonschema.ReflectContext option that sets the operation context (oc) in the reflect context (rc) usingcontext.WithValue. - The
OperationCtxfunction retrieves the operation context from the reflect context using theocCtxKeykey. - The
nolint:fatcontextcomment is used to disable the "fat context" linter, which might be triggered by the use ofcontext.WithValue.
- The
- LlamaPReview Suggested Improvements:
type ocCtxKey struct{}
// WithOperationCtx is a jsonschema.ReflectContext option.
func WithOperationCtx(oc OperationContext, isProcessingResponse bool, in In) func(rc *jsonschema.ReflectContext) {
return func(rc *jsonschema.ReflectContext) {
oc.SetIsProcessingResponse(isProcessingResponse)
oc.SetProcessingIn(in)
rc.Context = context.WithValue(rc.Context, ocCtxKey{}, oc)
}
}
// OperationCtx retrieves operation context from reflect context.
func OperationCtx(rc *jsonschema.ReflectContext) (OperationContext, bool) {
if oc, ok := rc.Value(ocCtxKey{}).(OperationContext); ok {
return oc, true
}
return nil, false
}- Improvement rationale:
- The suggested improvement removes the
nolint:fatcontextcomment, as it's not necessary and might be confusing to other developers.
- The suggested improvement removes the
- Technical benefits:
- Improved code readability by removing unnecessary comments.
- Business value:
- Better code maintainability and easier understanding for other developers.
- Risk assessment:
- The suggested improvement is a low-risk change, as it only involves removing an unnecessary comment. It doesn't change the behavior of the code or the system under test.
- operation.go - WithOperationCtx (Alternative approach)
- Submitted PR Code:
type ocCtxKey struct{}
// WithOperationCtx is a jsonschema.ReflectContext option.
func WithOperationCtx(oc OperationContext, isProcessingResponse bool, in In) func(rc *jsonschema.ReflectContext) {
return func(rc *jsonschema.ReflectContext) {
oc.SetIsProcessingResponse(isProcessingResponse)
oc.SetProcessingIn(in)
rc.Context = context.WithValue(rc.Context, ocCtxKey{}, oc) //nolint:fatcontext
}
}
// OperationCtx retrieves operation context from reflect context.
func OperationCtx(rc *jsonschema.ReflectContext) (OperationContext, bool) {
if oc, ok := rc.Value(ocCtxKey{}).(OperationContext); ok {
return oc, true
}
return nil, false
}- Alternative Approach:
- Instead of using
context.WithValueto set the operation context in the reflect context, an alternative approach could be to use a custom context type that embeds the operation context. - This approach might provide better type safety and avoid the need for using
context.WithValue.
- Instead of using
- LlamaPReview Suggested Improvements:
type operationCtx struct {
*jsonschema.ReflectContext
OperationContext
}
// WithOperationCtx is a jsonschema.ReflectContext option.
func WithOperationCtx(oc OperationContext, isProcessingResponse bool, in In) func(rc *jsonschema.ReflectContext) {
return func(rc *jsonschema.ReflectContext) {
oc.SetIsProcessingResponse(isProcessingResponse)
oc.SetProcessingIn(in)
return &operationCtx{ReflectContext: rc, OperationContext: oc}
}
}
// OperationCtx retrieves operation context from reflect context.
func OperationCtx(rc *jsonschema.ReflectContext) (OperationContext, bool) {
oc, ok := rc.(*operationCtx)
if ok {
return oc.OperationContext, true
}
return nil, false
}- Improvement rationale:
- The suggested alternative approach uses a custom
operationCtxstruct that embeds both the reflect context and the operation context. - This approach provides better type safety, as the operation context is now part of the reflect context type.
- It also avoids the need for using
context.WithValue, which might be confusing to other developers.
- The suggested alternative approach uses a custom
- Technical benefits:
- Improved type safety by embedding the operation context in the reflect context type.
- Better readability and easier understanding for other developers by avoiding the use of
context.WithValue.
- Business value:
- More maintainable code and easier understanding for other developers.
- Risk assessment:
- The suggested alternative approach is a medium-risk change, as it involves refactoring the code to use a custom context type. It's essential to thoroughly test the changes to ensure they don't introduce new issues or regressions.
2.2 Implementation Quality
- Code organization and structure: The code is well-organized, with clear separation of concerns and responsibilities. The use of packages and modules helps maintain a clean and modular codebase.
- Design patterns usage: The code employs appropriate design patterns, such as the use of interfaces and structs for data modeling, and the use of context for managing state and behavior.
- Error handling approach: The error handling approach is consistent and thorough, with proper use of error types and appropriate handling of errors throughout the codebase.
- Resource management: The code demonstrates good resource management practices, with proper cleanup and disposal of resources when they are no longer needed.
- Performance considerations: The code includes performance optimizations, such as the use of caching and efficient data structures, to ensure optimal performance under various workloads.
2.3 Critical Findings
Details
Potential Issues
-
🔴 Critical Issues
- Issue description: The fix for self-referential structs might not cover all edge cases, as the tests only cover a limited set of scenarios.
- Impact: If the fix doesn't handle all edge cases, it might still result in panics or unexpected behaviors when dealing with self-referential structs.
- Recommendation: Thoroughly test the fix with various edge cases, including empty or null values, to ensure it handles these cases gracefully. Consider adding more test cases to cover a broader range of scenarios.
-
🟡 Warnings
- Warning description: The updated
refldependency might introduce new behaviors or edge cases that are not yet tested or understood. - Potential risks: The new behaviors or edge cases might cause unexpected issues or incompatibilities with other parts of the system.
- Suggested improvements:
- Thoroughly test the changes with various scenarios and edge cases to ensure they don't have unintended side effects on other parts of the system.
- Review the changes with a security lens to ensure they don't introduce new security vulnerabilities.
- Benchmark the changes to ensure they don't introduce significant performance regressions.
- Warning description: The updated
Code Quality Concerns
- Maintainability aspects: The code is generally well-maintained, with clear and concise comments and consistent coding styles. However, some parts of the codebase could benefit from additional refactoring or optimization to improve maintainability and readability.
- Readability issues: Some parts of the codebase might be difficult to understand for developers who are not familiar with the system or the specific domain. Improving the documentation and adding more comments can help address this concern.
- Performance bottlenecks: While the code includes performance optimizations, there might still be opportunities to improve performance, especially in high-load or real-time scenarios. Profiling and benchmarking the code can help identify and address performance bottlenecks.
2.4 Security Assessment
Details
Authentication/Authorization impacts
- Potential risks: The fix for self-referential structs might introduce new authentication or authorization risks if not properly secured. It's essential to ensure that the fix doesn't expose sensitive data or allow unauthorized access to protected resources.
- Mitigation strategies:
- Thoroughly test the fix with various authentication and authorization scenarios to ensure it doesn't introduce new vulnerabilities.
- Review the changes with a security lens to ensure they adhere to best security practices and industry standards.
Data handling concerns
- Potential risks: The fix might introduce new data handling risks, such as data leakage or data tampering, if not properly secured. It's essential to ensure that the fix doesn't expose sensitive data or allow unauthorized access to protected resources.
- Mitigation strategies:
- Thoroughly test the fix with various data handling scenarios to ensure it doesn't introduce new vulnerabilities.
- Review the changes with a security lens to ensure they adhere to best security practices and industry standards.
Input validation
- Potential risks: The fix might introduce new input validation risks if not properly secured. It's essential to ensure that the fix doesn't expose sensitive data or allow unauthorized access to protected resources.
- Mitigation strategies:
- Thoroughly test the fix with various input validation scenarios to ensure it doesn't introduce new vulnerabilities.
- Review the changes with a security lens to ensure they adhere to best security practices and industry standards.
Security best practices
- Potential risks: The fix might introduce new security risks if not properly secured. It's essential to ensure that the fix adheres to best security practices and industry standards.
- Mitigation strategies:
- Thoroughly test the fix with various security scenarios to ensure it doesn't introduce new vulnerabilities.
- Review the changes with a security lens to ensure they adhere to best security practices and industry standards.
Potential security risks
- Potential risks: The fix might introduce new security risks if not properly secured. It's essential to ensure that the fix adheres to best security practices and industry standards.
- Mitigation strategies:
- Thoroughly test the fix with various security scenarios to ensure it doesn't introduce new vulnerabilities.
- Review the changes with a security lens to ensure they adhere to best security practices and industry standards.
Mitigation strategies
- Mitigation strategies:
- Thoroughly test the fix with various security scenarios to ensure it doesn't introduce new vulnerabilities.
- Review the changes with a security lens to ensure they adhere to best security practices and industry standards.
- Benchmark the changes to ensure they don't introduce significant performance regressions.
2.5 Testing Strategy
Details
Test Coverage
- Unit test analysis:
- The PR includes unit tests for the changes, focusing on the fix for self-referential structs and the updated
refldependency. - The unit tests cover a limited set of scenarios, and additional tests are needed to ensure comprehensive coverage.
- The PR includes unit tests for the changes, focusing on the fix for self-referential structs and the updated
- Integration test requirements:
- Integration tests are required to validate the fix in various scenarios and edge cases, especially for handling self-referential structs and interacting with other components of the system.
- Integration tests should cover a broader range of scenarios, including edge cases and potential failure points.
- Edge cases coverage:
- The current unit tests do not cover all edge cases, and additional tests are needed to ensure comprehensive coverage.
- Integration tests should cover a broader range of scenarios, including edge cases and potential failure points.
Test Recommendations
Suggested Test Cases
// Sample test case for handling empty SubEntity
func TestSelfReferenceEmptySubEntity(t *testing.T) {
reflector := openapi3.NewReflector()
type SubEntity struct {
Self *SubEntity `json:"self"`
}
type My struct {
Foo string `json:"foo"`
SubEntity *SubEntity `json:"subentity"`
}
putOp, err := reflector.NewOperationContext(http.MethodPut, "/things/")
require.NoError(t, err)
putOp.AddReqStructure(My{})
putOp.AddRespStructure(My{})
require.NoError(t, reflector.AddOperation(putOp))
expectedSchema := `{
"openapi":"3.0.3","info":{"title":"","version":""},
"paths":{
"/things/":{
"put":{
"requestBody":{
"content":{
"application/json":{"schema":{"$ref":"#/components/schemas/Openapi3TestMy"}}
}
},
"responses":{
"200":{
"description":"OK",
"content":{
"application/json":{"schema":{"$ref":"#/components/schemas/Openapi3TestMy"}}
}
}
}
}
}
},
"components":{
"schemas":{
"Openapi3TestMy":{
"type":"object",
"properties":{
"foo":{"type":"string"},
"subentity":{"$ref":"#/components/schemas/Openapi3TestSubEntity"}
}
},
"Openapi3TestSubEntity":{
"type":"object"
}
}
}
}`
assertjson.EqMarshal(t, expectedSchema, reflector.SpecSchema())
}- Coverage improvements:
- Add more unit tests to cover a broader range of scenarios, including edge cases and potential failure points.
- Implement integration tests to validate the fix in various scenarios and edge cases, especially for handling self-referential structs and interacting with other components of the system.
- Performance testing needs:
- Benchmark the changes to ensure they don't introduce significant performance regressions.
- Profile the code to identify and address performance bottlenecks.
2.6 Documentation & Maintenance
Details
Documentation updates needed
- API documentation: The PR updates the OpenAPI documentation generated by the library, and the API documentation should be updated to reflect these changes.
- Architecture documentation: The fix for self-referential structs might have implications for the system architecture, and the architecture documentation should be updated to reflect these changes.
- Configuration documentation: The updated
refldependency might require updates to the configuration documentation, especially if there are new configuration options or behaviors introduced by the updated dependency.
Long-term maintenance considerations
- Technical debt and monitoring requirements:
- The fix for self-referential structs might introduce new technical debt, such as additional tests or refactoring, that needs to be addressed over time.
- Monitoring the system for potential regressions or new issues introduced by the fix is essential to ensure long-term stability and reliability.
- Monitoring requirements:
- Implement monitoring and alerting for potential regressions or new issues introduced by the fix.
- Regularly review and update the monitoring strategy to ensure it remains effective and relevant.
2.7 Deployment & Operations
Details
Deployment impact and strategy
- Potential deployment impact: The fix for self-referential structs might have implications for the deployment process, especially if there are changes to the deployment infrastructure or configuration.
- Deployment strategy:
- Thoroughly test the changes in various deployment scenarios to ensure they don't introduce new issues or regressions.
- Implement a rollback plan in case the changes cause unexpected issues or regressions in production.
Key operational considerations
- Operational impact: The fix for self-referential structs might have implications for the system's operational behavior, especially if there are changes to the system's configuration or state management.
- Operational strategy:
- Monitor the system for potential regressions or new issues introduced by the fix.
- Implement operational procedures to handle any unexpected issues or regressions that might arise from the changes.
3. Summary & Recommendations
3.1 Key Action Items
- Critical changes required:
- Thoroughly test the fix for self-referential structs with various edge cases, including empty or null values, to ensure it handles these cases gracefully.
- Implement integration tests to validate the fix in various scenarios and edge cases, especially for handling self-referential structs and interacting with other components of the system.
- Review the changes with a security lens to ensure they adhere to best security practices and industry standards.
- Benchmark the changes to ensure they don't introduce significant performance regressions.
- Important improvements suggested:
- Add more unit tests to cover a broader range of scenarios, including edge cases and potential failure points.
- Update the documentation to reflect the changes, including API documentation, architecture documentation, and configuration documentation.
- Implement monitoring and alerting for potential regressions or new issues introduced by the fix.
- Best practices to implement:
- Follow best practices for error handling, resource management, and performance considerations.
- Regularly review and update the system's documentation and monitoring strategy to ensure they remain effective and relevant.
3.2 Future Considerations
- Technical evolution path: The fix for self-referential structs might have implications for the system's technical evolution path, especially if there are plans to introduce new features or functionality that rely on self-referential structs.
- Business capability evolution: The fix might have implications for the system's business capabilities, especially if there are plans to expand or modify the system's functionality to support new business requirements.
- System integration impacts: The fix might have implications for the system's integration with other components or systems, especially if there are plans to modify or extend the system's integration points.
💡 LlamaPReview Community
Have feedback on this AI Code review tool? Join our GitHub Discussions to share your thoughts and help shape the future of LlamaPReview.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #137.