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

Sort HTTP methods to remove randomness from keys #149

Merged
merged 2 commits into from
Sep 11, 2023

Conversation

Chewie69006
Copy link
Contributor

Hello !

As data.keys returns in a random order, comparing 2 arrays containing same objects in different order make it return false. This way, it will return true as they will be sorted.

Copy link
Contributor

@AvdLee AvdLee left a comment

Choose a reason for hiding this comment

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

Great catch! Would it be possible to write a unit test for this as well? That way, we prevent it from happening again in the future.

@wetransferplatform
Copy link
Collaborator

wetransferplatform commented Sep 4, 2023

Warnings
⚠️

Consider to place some MARK: lines for Sources/Mocker/Mock.swift, which is over 300 lines big.

Messages
📖

View more details on Bitrise

SwiftLint found issues

Severity File Reason
Warning MockTests.swift:10 Imports should be sorted (sorted_imports)
Warning MockTests.swift:35 Lines should not have trailing whitespace (trailing_whitespace)
Warning MockTests.swift:38 Lines should not have trailing whitespace (trailing_whitespace)
Warning Mock.swift:9 The disabled 'force_unwrapping' rule should be re-enabled before the end of the file (blanket_disable_command)
Warning Mock.swift:93 Computed properties should first declare the getter and then the setter (computed_accessors_order)
Error Mock.swift:111 Prefer checking isEmpty over comparing count to zero (empty_count)
Error Mock.swift:8 Line should be 160 characters or less; currently it has 177 characters (line_length)
Error Mock.swift:38 Line should be 160 characters or less; currently it has 195 characters (line_length)
Warning Mock.swift:55 Line should be 140 characters or less; currently it has 155 characters (line_length)
Error Mock.swift:87 Line should be 160 characters or less; currently it has 211 characters (line_length)
Error Mock.swift:90 Line should be 160 characters or less; currently it has 208 characters (line_length)
Error Mock.swift:101 Line should be 160 characters or less; currently it has 217 characters (line_length)
Error Mock.swift:110 Line should be 160 characters or less; currently it has 297 characters (line_length)
Warning Mock.swift:116 Line should be 140 characters or less; currently it has 152 characters (line_length)
Warning Mock.swift:156 Line should be 140 characters or less; currently it has 141 characters (line_length)
Error Mock.swift:159 Line should be 160 characters or less; currently it has 171 characters (line_length)
Warning Mock.swift:178 Line should be 140 characters or less; currently it has 141 characters (line_length)
Warning Mock.swift:185 Line should be 140 characters or less; currently it has 142 characters (line_length)
Error Mock.swift:186 Line should be 160 characters or less; currently it has 246 characters (line_length)
Warning Mock.swift:204 Line should be 140 characters or less; currently it has 141 characters (line_length)
Error Mock.swift:206 Line should be 160 characters or less; currently it has 171 characters (line_length)
Error Mock.swift:211 Line should be 160 characters or less; currently it has 256 characters (line_length)
Warning Mock.swift:234 Line should be 140 characters or less; currently it has 150 characters (line_length)
Error Mock.swift:249 Line should be 160 characters or less; currently it has 171 characters (line_length)
Warning Mock.swift:253 Line should be 140 characters or less; currently it has 160 characters (line_length)
Warning Mock.swift:268 Line should be 140 characters or less; currently it has 141 characters (line_length)
Error Mock.swift:270 Line should be 160 characters or less; currently it has 171 characters (line_length)
Error Mock.swift:275 Line should be 160 characters or less; currently it has 262 characters (line_length)
Warning Mock.swift:326 Line should be 140 characters or less; currently it has 159 characters (line_length)
Error Mock.swift:336 Line should be 160 characters or less; currently it has 168 characters (line_length)
Warning Mock.swift:129 Use shorthand syntax for optional binding (shorthand_optional_binding)
Warning Mock.swift:338 Use shorthand syntax for optional binding (shorthand_optional_binding)
Warning Mock.swift:338 Use shorthand syntax for optional binding (shorthand_optional_binding)
Warning Mock.swift:42 Lines should not have trailing whitespace (trailing_whitespace)
Warning Mock.swift:114 Lines should not have trailing whitespace (trailing_whitespace)
Warning Mock.swift:155 Lines should not have trailing whitespace (trailing_whitespace)
Warning Mock.swift:199 Lines should not have trailing whitespace (trailing_whitespace)
Warning Mock.swift:224 Lines should not have trailing whitespace (trailing_whitespace)
Warning Mock.swift:244 Lines should not have trailing whitespace (trailing_whitespace)
Warning Mock.swift:263 Lines should not have trailing whitespace (trailing_whitespace)

Generated by 🚫 Danger Swift against e08d07a

@AvdLee
Copy link
Contributor

AvdLee commented Sep 5, 2023

@Chewie69006 thanks for writing the test, great job! Let's wait for one more review and we can merge.

Copy link
Contributor

@BasThomas BasThomas left a comment

Choose a reason for hiding this comment

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

Great!

@BasThomas
Copy link
Contributor

Looks like your commit adding the test isn't signed, which blocks us from merging. Can you take care of that, @Chewie69006? Let us know if we can help.

@AvdLee AvdLee merged commit d56222c into WeTransfer:master Sep 11, 2023
@Chewie69006
Copy link
Contributor Author

When do you think a new tag will be delivered ?

@Chewie69006 Chewie69006 deleted the patch-1 branch September 22, 2023 13:23
@wetransferplatform
Copy link
Collaborator

Congratulations! 🎉 This was released as part of Release 3.0.2 🚀

Generated by GitBuddy

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.

4 participants