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

Update TravisCI: Remove Swift 4.x; Add Swift 5.4 #61

Merged
merged 3 commits into from
Aug 23, 2021

Conversation

dannys42
Copy link
Contributor

No description provided.

@dannys42 dannys42 closed this Aug 22, 2021
@dannys42 dannys42 reopened this Aug 22, 2021
@dannys42
Copy link
Contributor Author

@mbarnach @adam-rocska What are your thoughts on the changes here? Apparently there's a minor change in JSONEncoder between Swift 4.x and Swift 5.x and JSON fragments are now supported. But I think KituraContracts was relying on fragments not being supported. It's a little inefficient, but I just threw in an extra check as I saw no option for JSONEncoder to disable allowing fragments.

@mbarnach
Copy link
Member

🤔 Interesting... Are you sure this is problematic? Or more specifically, in which way was it problematic for you?

@dannys42
Copy link
Contributor Author

I'm not entirely sure it's a problem... just that there was a unit test explicitly testing that error responses must not be JSON fragments. So in the absence of more insight, I'm attempting to preserve that behavior. Unfortunately the error that is thrown is not preserved, but I'm hoping that won't be an issue for any practical uses of the framework.

@mbarnach
Copy link
Member

I think that's the expected behaviour. I cannot find a quick and easy way to do it without that extra test, but I'm wondering if we shouldn't put it under Debug or any other flag, as it is quite "intense" otherwise?

@dannys42
Copy link
Contributor Author

@mbarnach yah, that sounds reasonable.

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 22 Code Smells

No Coverage information No Coverage information
97.0% 97.0% Duplication

Copy link
Member

@mbarnach mbarnach left a comment

Choose a reason for hiding this comment

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

All good to me!

@dannys42 dannys42 merged commit 6765033 into Kitura:master Aug 23, 2021
@dannys42 dannys42 deleted the MISC-CI-remove_swift4x branch August 23, 2021 22:11
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.

2 participants