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

1.2 dev branch #192

Merged
merged 41 commits into from Apr 9, 2020
Merged

1.2 dev branch #192

merged 41 commits into from Apr 9, 2020

Conversation

guyacosta
Copy link
Contributor

Unit tests Module
• 189 passing tests added covering both NuGet and CLI entry points under separate namespaces/classes
• Improved overall code quality and reporting of invalid args/combinations

Overall Code Structure
• Refactoring+rework for technical debt getting to 1.0 eliminating unused code, renaming, moving to isolate to right project
• *Object results-based support for NuGet callers rather than json/text strings
• *New support for First/Best match options for results (best match elevates higher confidence value matches)
• Separation of CLI vs Operation argument options
• All file output format and writters moved to CLI project
• Cleaner isolation of functionality and readability
• Text and Json output support added for all commands (previously limited to text for all but analyze)
• Better use of inheritance to organize common arguments and override
• Better use of polymorphism for checking arguments

Resolves #191

@guyacosta guyacosta self-assigned this Apr 6, 2020
@guyacosta
Copy link
Contributor Author

guyacosta commented Apr 6, 2020

Suggest to download and review PR branch in it's entirely due to the size of change.

Notes:
Detailed Code Changes
CLI Module
• All file writers moved to this project out of command/core module that return objects only
• All command options include output args
• All common options in base class
• Check for output file path write before running op
• Checks added for log write ability to specified path before operation starts
Command Module
• All arguments exclude CLI only output options
• All write statements related to output removed = no IO other than log
• GetResult() method for each command returning Results object not json string
• Result objects include standard result code and embedded objects that are serializable and only contain methods specific to the result data including the AppProfile class which is gone now
• Renaming of classes, properties, variables for readabilty, accuracy e.g. MsgHelp, not ErrMsg, ScanResult not Issue in RulesEngine
• Tag counter support outside of CLI entry point; no dependency on preferences using "Metric" tag type as key
RulesEngine Module
• *Added support for First vs Best match (perf/quality) with AllowDuplicates option

.editorconfig Outdated Show resolved Hide resolved
.editorconfig Outdated Show resolved Hide resolved
AppInspector.CLI/CLICmdOptions.cs Show resolved Hide resolved
AppInspector.CLI/CLICmdOptions.cs Outdated Show resolved Hide resolved
AppInspector.CLI/ResultsWriter.cs Outdated Show resolved Hide resolved
UnitTest.Commands/source/empty.json Outdated Show resolved Hide resolved
UnitTest.Commands/source/empty.cpp Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
NewDesignExample.cs Outdated Show resolved Hide resolved
guyacosta and others added 2 commits April 7, 2020 09:19
Co-Authored-By: Gabe Stocco <98900+gfs@users.noreply.github.com>
Co-Authored-By: Gabe Stocco <98900+gfs@users.noreply.github.com>
AppInspector.CLI/Program.cs Show resolved Hide resolved
AppInspector.CLI/Program.cs Show resolved Hide resolved
AppInspector.CLI/Program.cs Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Apr 7, 2020

This pull request introduces 9 alerts and fixes 3 when merging 451f109 into 642513b - view on LGTM.com

new alerts:

  • 5 for Useless assignment to local variable
  • 2 for Container contents are never accessed
  • 1 for Poor error handling: empty catch block
  • 1 for Dereferenced variable may be null

fixed alerts:

  • 3 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Apr 7, 2020

This pull request introduces 9 alerts and fixes 3 when merging d9ba137 into 642513b - view on LGTM.com

new alerts:

  • 5 for Useless assignment to local variable
  • 2 for Container contents are never accessed
  • 1 for Poor error handling: empty catch block
  • 1 for Dereferenced variable may be null

fixed alerts:

  • 3 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Apr 7, 2020

This pull request introduces 1 alert and fixes 3 when merging 6717e8a into 642513b - view on LGTM.com

new alerts:

  • 1 for Container contents are never accessed

fixed alerts:

  • 3 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Apr 7, 2020

This pull request introduces 1 alert and fixes 3 when merging f17fb75 into 642513b - view on LGTM.com

new alerts:

  • 1 for Container contents are never accessed

fixed alerts:

  • 3 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Apr 7, 2020

This pull request introduces 1 alert and fixes 3 when merging 899f2da into 642513b - view on LGTM.com

new alerts:

  • 1 for Container contents are never accessed

fixed alerts:

  • 3 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Apr 8, 2020

This pull request introduces 4 alerts and fixes 3 when merging 9583290 into 642513b - view on LGTM.com

new alerts:

  • 2 for Useless assignment to local variable
  • 1 for Container contents are never accessed
  • 1 for Missing Dispose call on local IDisposable

fixed alerts:

  • 3 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Apr 8, 2020

This pull request introduces 4 alerts and fixes 3 when merging 94fc0e7 into 642513b - view on LGTM.com

new alerts:

  • 2 for Useless assignment to local variable
  • 1 for Container contents are never accessed
  • 1 for Missing Dispose call on local IDisposable

fixed alerts:

  • 3 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Apr 8, 2020

This pull request introduces 4 alerts and fixes 3 when merging e3c61a7 into 642513b - view on LGTM.com

new alerts:

  • 2 for Useless assignment to local variable
  • 1 for Container contents are never accessed
  • 1 for Missing Dispose call on local IDisposable

fixed alerts:

  • 3 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Apr 8, 2020

This pull request introduces 4 alerts and fixes 3 when merging 4756e53 into 642513b - view on LGTM.com

new alerts:

  • 2 for Useless assignment to local variable
  • 1 for Container contents are never accessed
  • 1 for Missing Dispose call on local IDisposable

fixed alerts:

  • 3 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Apr 8, 2020

This pull request introduces 4 alerts and fixes 3 when merging cbed817 into 642513b - view on LGTM.com

new alerts:

  • 2 for Useless assignment to local variable
  • 1 for Container contents are never accessed
  • 1 for Missing Dispose call on local IDisposable

fixed alerts:

  • 3 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Apr 8, 2020

This pull request introduces 4 alerts and fixes 3 when merging 7041787 into 642513b - view on LGTM.com

new alerts:

  • 2 for Useless assignment to local variable
  • 1 for Container contents are never accessed
  • 1 for Missing Dispose call on local IDisposable

fixed alerts:

  • 3 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Apr 8, 2020

This pull request introduces 4 alerts and fixes 3 when merging 109fcc9 into 642513b - view on LGTM.com

new alerts:

  • 2 for Useless assignment to local variable
  • 1 for Container contents are never accessed
  • 1 for Missing Dispose call on local IDisposable

fixed alerts:

  • 3 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Apr 9, 2020

This pull request introduces 4 alerts and fixes 3 when merging 68b8607 into 6dca8d1 - view on LGTM.com

new alerts:

  • 2 for Useless assignment to local variable
  • 1 for Container contents are never accessed
  • 1 for Missing Dispose call on local IDisposable

fixed alerts:

  • 3 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Apr 9, 2020

This pull request introduces 4 alerts and fixes 3 when merging 1e80738 into 6dca8d1 - view on LGTM.com

new alerts:

  • 2 for Useless assignment to local variable
  • 1 for Container contents are never accessed
  • 1 for Missing Dispose call on local IDisposable

fixed alerts:

  • 3 for Useless assignment to local variable

@guyacosta guyacosta merged commit 921413f into master Apr 9, 2020
@guyacosta guyacosta deleted the 1.2DevBranch branch April 9, 2020 19:12
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.

Return result objects not json/text for NuGet calls
3 participants