Skip to content

Commit

Permalink
Create issues for TODOs
Browse files Browse the repository at this point in the history
  • Loading branch information
jameswinkler committed Jan 26, 2021
1 parent 74c7c74 commit 263e636
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 0 deletions.
1 change: 1 addition & 0 deletions src/Sarif.Driver/DriverExtensionMethods.cs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ public static bool ValidateOutputOptions(this MultipleFilesOptionsBase options)
bool valid = true;

// TODO: Is it correct to modify options in a "validate" method?
// #2267 https://github.com/microsoft/sarif-sdk/issues/2267
if (options.Inline)
{
options.Force = true;
Expand Down
2 changes: 2 additions & 0 deletions src/Sarif.Driver/Sdk/CommandBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public abstract class CommandBase

// TODO: Removing this constructor broke the one of AbsoluteUriCommand entirely but all unit tests were passing
// Add unit tests that will break when this constructor is deleted or malfunctioning.
// #2268 https://github.com/microsoft/sarif-sdk/issues/2268
public CommandBase(IFileSystem fileSystem = null)
{
this.FileSystem = fileSystem ?? Sarif.FileSystem.Instance;
Expand All @@ -28,6 +29,7 @@ public CommandBase(IFileSystem fileSystem = null)
// TODO: What's the point of having a bunch of static methods in an abstract class?
// We even have a static class, "CommandUtilities" which seems like the more appropriate
// place for these to go.
// #2269 https://github.com/microsoft/sarif-sdk/issues/2269
protected static bool ValidateNonNegativeCommandLineOption<T>(long optionValue, string optionName)
{
bool valid = true;
Expand Down
2 changes: 2 additions & 0 deletions src/Sarif.Multitool.Library/RewriteCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ private bool ValidateOptions(RewriteOptions rewriteOptions)

// While this is returning true for inline cases, I think it's doing so for the wrong reasons.
// TODO: validate whether "actualOutputPath" can be created.
// #2270 https://github.com/microsoft/sarif-sdk/issues/2270
if (!DriverUtilities.ReportWhetherOutputFileCanBeCreated(rewriteOptions.OutputFilePath, rewriteOptions.Force, _fileSystem))
{
return false;
Expand All @@ -101,6 +102,7 @@ private bool ValidateOptions(RewriteOptions rewriteOptions)
}

// TODO Move this into a separate class for better unit testing
// #2271 https://github.com/microsoft/sarif-sdk/issues/2271
private string SniffVersion(string sarifPath)
{
using (JsonTextReader reader = new JsonTextReader(new StreamReader(_fileSystem.FileOpenRead(sarifPath))))
Expand Down
1 change: 1 addition & 0 deletions src/Sarif/Visitors/SarifCurrentToVersionOneVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public override SarifLog VisitSarifLog(SarifLog v2SarifLog)
}

// TODO: We always return null here. Is there a pattern that allows us to change the method signature to "void" ?
// #2266 https://github.com/microsoft/sarif-sdk/issues/2266
return null;
}

Expand Down

0 comments on commit 263e636

Please sign in to comment.