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

chore(core): CNX-7435 Core warning reduction #3082

Merged
merged 22 commits into from
Dec 1, 2023

Conversation

JR-Morgan
Copy link
Member

AlanRynne and others added 7 commits November 28, 2023 16:38
* fix(core): IDE0005 Redundant Using statements

* fix(ci): GenerateDocumentationFile is required for IDE0005 to run on CI

* fix(ci): Allow warnings on all CI jobs
* fix(gh): IDE0005 for Grasshopper connector

* fix(rhino): IDE0005 for Rhino connector

* fix(rh/gh): IDE0005 for Rhino/Grasshopper shared converter
@JR-Morgan JR-Morgan requested a review from AlanRynne November 30, 2023 15:18
@JR-Morgan JR-Morgan changed the base branch from main to dev November 30, 2023 15:18
@JR-Morgan JR-Morgan changed the title Jrm/core/warning elimination chore(core): Core warnings Nov 30, 2023
@JR-Morgan JR-Morgan marked this pull request as ready for review November 30, 2023 18:15
@JR-Morgan
Copy link
Member Author

I want to give this some quick testing in some connectors and automate. All the changes I've made should be pretty innocent, but there's so many, it's worth being cautious.

@JR-Morgan JR-Morgan force-pushed the jrm/core/warning-elimination branch from 2343309 to 5ca04e8 Compare December 1, 2023 00:08
@JR-Morgan JR-Morgan force-pushed the jrm/core/warning-elimination branch from 5ca04e8 to 8449062 Compare December 1, 2023 00:08
Copy link
Contributor

@AlanRynne AlanRynne left a comment

Choose a reason for hiding this comment

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

Added some single comments before this review by accident, and a couple more within this 👇🏼

Comment on lines +21 to +33
private const int BATCH_SIZE_GET_OBJECTS = 10000;
private const int BATCH_SIZE_HAS_OBJECTS = 100000;

private const int MAX_MULTIPART_COUNT = 5;
private const int MAX_MULTIPART_SIZE = 25_000_000;
private const int MAX_OBJECT_SIZE = 25_000_000;

private const int MAX_REQUEST_SIZE = 100_000_000;

private const int RETRY_COUNT = 3;
private static readonly HashSet<int> s_retryCodes = new() { 408, 502, 503, 504 };
private static readonly char[] s_separator = { '\t' };
private static readonly string[] s_filenameSeparator = { "filename=" };
Copy link
Contributor

Choose a reason for hiding this comment

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

We should consider moving these to a "magic numbers" class at some point, as ideally all these values should be defined by the server.

@AlanRynne AlanRynne changed the title chore(core): Core warnings chore(core): CNX7435 Core warning reduction Dec 1, 2023
@AlanRynne AlanRynne changed the title chore(core): CNX7435 Core warning reduction chore(core): CNX-7435 Core warning reduction Dec 1, 2023
@AlanRynne AlanRynne merged commit 93f95f4 into dev Dec 1, 2023
@AlanRynne AlanRynne deleted the jrm/core/warning-elimination branch December 1, 2023 18:40
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