-
Notifications
You must be signed in to change notification settings - Fork 85
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
Fix warnings mixed #941
Conversation
…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.
@@ -59,7 +59,8 @@ private enum LogLevel { | |||
File.Delete(LogFilename); | |||
} | |||
} | |||
catch (Exception) { } | |||
catch (Exception) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
that should be part of complete logging overhaul i think. |
There was a problem hiding this 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
had to fix a merge conflict. |
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.