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

Fix warnings mixed #941

Merged
merged 3 commits into from
Jun 15, 2020
Merged

Fix warnings mixed #941

merged 3 commits into from
Jun 15, 2020

Conversation

DaEgi01
Copy link
Contributor

@DaEgi01 DaEgi01 commented Jun 12, 2020

Deleted Functional.cs and IVisitor.cs. This stuff is not used and although interesting is implemented badly anyway (classes instead of structs).

Deleted LogRet method since its not used too.

Removed a lot of warnings that had only single digit counts where a separate PR would be too much.

SA1626 Single-line comments should not use documentation style slashes.
SA1617 Void return value should not be documented.
SA1613 Element parameter documentation should declare parameter name.
SA1642 Constructor summary documentation should begin with standard text.
SA1649 File name should match first type name.
SA1520 Use braces consistently.
SA1604 Element documentation should have summary.
CS0105 The using directive for 'X' appeared previously in this namespace.
SA1003 Operator 'X' should not be preceded by whitespace.
SA1006 Preprocessor keyword 'endif' should not be preceded by a space.
SA1517 Code should not contain blank lines at start of file.
SA1509 Opening braces should not be preceded by blank line.
SA1506 Element documentation headers should not be followed by blank line.
SA1413 Use trailing comma in multi-line initializers.
SA1314 Type parameter names should begin with T.
SA1001 Commas should be followed by whitespace.
SA1002 Semicolons should be followed by a space.
SA1002 Semicolons should not be preceded by a space.
SA1501 Statement should not be on a single line.
SA1303 Const field names should begin with upper-case letter.
SA1135 Using directive for namespace 'X' should be qualified.
SA1129 Do not use default value type constructor.
SA1122 Use string.Empty for empty strings.
SA1110 Opening parenthesis or bracket should be on declaration line.
SA1108 Block statements should not contain embedded comments.
SA1104 Query clause should begin on new line when previous clause spans multiple lines.

…e functional stuff although interesting is implemented badly (classes instead of structs).

Deleted LogRet<T> method since its not used too.

Removed a lot of warnings that had only single digit counts where a separate PR would be too much.

SA1626	Single-line comments should not use documentation style slashes.
SA1617	Void return value should not be documented.
SA1613	Element parameter documentation should declare parameter name.
SA1642	Constructor summary documentation should begin with standard text.
SA1649	File name should match first type name.
SA1520	Use braces consistently.
SA1604	Element documentation should have summary.
CS0105	The using directive for 'X' appeared previously in this namespace.
SA1003	Operator 'X' should not be preceded by whitespace.
SA1006	Preprocessor keyword 'endif' should not be preceded by a space.
SA1517	Code should not contain blank lines at start of file.
SA1509	Opening braces should not be preceded by blank line.
SA1506	Element documentation headers should not be followed by blank line.
SA1413	Use trailing comma in multi-line initializers.
SA1314	Type parameter names should begin with T.
SA1001	Commas should be followed by whitespace.
SA1002	Semicolons should be followed by a space.
SA1002	Semicolons should not be preceded by a space.
SA1501	Statement should not be on a single line.
SA1303	Const field names should begin with upper-case letter.
SA1135	Using directive for namespace 'X' should be qualified.
SA1129	Do not use default value type constructor.
SA1122	Use string.Empty for empty strings.
SA1110	Opening parenthesis or bracket should be on declaration line.
SA1108	Block statements should not contain embedded comments.
SA1104	Query clause should begin on new line when previous clause spans multiple lines.
@originalfoo originalfoo added the code cleanup Refactor code, remove old code, improve maintainability label Jun 12, 2020
@originalfoo originalfoo added this to the 11.6.0 milestone Jun 12, 2020
@@ -59,7 +59,8 @@ private enum LogLevel {
File.Delete(LogFilename);
}
}
catch (Exception) { }
catch (Exception) {
Copy link
Member

Choose a reason for hiding this comment

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

Is there even need for the Exception here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only wanted to fix the warning, not change behavior.
Personally I think such global "Ignore all exceptions" blocks are a no go.

Copy link
Member

Choose a reason for hiding this comment

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

I guess in this instance it would be pointless to try logging an exception (: Unless it was pumped to Debug.Log(), which might also fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Either pointless, or it would destroy the previous logfile, or it would just defer the exception until LogToFile is called. We actually have no clue, since we don't know what caused it. We told it to ignore anything that can happen, while not handling anything at all.

Copy link
Member

Choose a reason for hiding this comment

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

Well, if our log file is failing for some reason, we can't use it. So only alternative is Unity Debug.Log(); at least then we have some chance of learning about the error.

Copy link
Contributor Author

@DaEgi01 DaEgi01 Jun 12, 2020

Choose a reason for hiding this comment

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

ohh, you are talking about mitigations?
yeah sure debug.log would be a valid strategy. i m just saying that hiding errors under the carpet does not fix them. in that case i would always default to letting the exeption bubble up instead. everything else just causes more pain in the end, at least that is my experience.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, can we Debug.Log() the exception then? I know it's not specifically related to this PR but it doesn't really merit a separate PR.

@DaEgi01
Copy link
Contributor Author

DaEgi01 commented Jun 13, 2020

that should be part of complete logging overhaul i think.

Copy link
Member

@originalfoo originalfoo left a comment

Choose a reason for hiding this comment

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

Ok, LGTM 👍

# Conflicts:
#	TLM/TLM/UI/UITransportDemand.cs
@DaEgi01
Copy link
Contributor Author

DaEgi01 commented Jun 15, 2020

had to fix a merge conflict.

@DaEgi01 DaEgi01 merged commit 7b7bc45 into master Jun 15, 2020
@DaEgi01 DaEgi01 deleted the FixWarnings_Mixed branch June 15, 2020 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code cleanup Refactor code, remove old code, improve maintainability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants