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

Detect unsupported tools in projects #237

Merged
merged 8 commits into from
Feb 16, 2022
Merged

Conversation

ofalvai
Copy link
Contributor

@ofalvai ofalvai commented Feb 15, 2022

Context

Detect unsupported tools/frameworks/platforms in the project directory to log them later.

Changes

  • Create scanner/unknown.go for everything related to scanning the repo for unsupported but recognized tools. This is not a scanner though, because it needs to run only after all other platform scanners fail.
  • A common interface for the various tool detectors and 3 actual implementations:
    • singleFileToolDetector that looks for only a single file name
    • multiFileToolDetector that first looks for a primary file name, but can also find one of the optional files as a fallback
    • kotlinMultiplatformDetector because there is no distinct file name pattern for KMM, so it also looks at the file contents of certain files 🤷
  • While traversing the folders recursively, the detectors also build a string representation of the file tree for logging purposes. Similar to the tree command, but a bit simpler.

Decisions

  • Maximum scan depth is 3
  • A list of excluded directories is defined in code

Out of scope for this PR

  • Wiring this into the project scanner flow
  • Logging of results

@ofalvai ofalvai changed the title Detect unsupported tools in projects [WIP] Detect unsupported tools in projects Feb 15, 2022
ProjectTree string
}

var UnknownToolDetectors = []UnknownToolDetector{
Copy link
Contributor

Choose a reason for hiding this comment

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

Bazel is missing from the list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed!

"Framework",
}

type singleFileToolDetector struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

The singleFileToolDetector and multiFileToolDetector implementations are too similar to each other which made me think we could unify them. Like a singleFileToolDetector can be represented with a multiFileToolDetector with a single required item and an empty optional items list.

And if we do that then we can drop the first part of its name and just keep toolDetector.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good idea actually!

"Pods",
"Carthage",
"CordovaLib",
"Framework",
Copy link
Contributor Author

@ofalvai ofalvai Feb 15, 2022

Choose a reason for hiding this comment

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

@tothszabi is this the correct folder name for iOS frameworks? I couldn't find any good info about this one folder name.

Copy link
Contributor

@tothszabi tothszabi Feb 15, 2022

Choose a reason for hiding this comment

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

Good that you asked this question because I have missed this. At the moment we use .framework and I think we should use that instead of Framework.

Have look if you could reuse these existing filters.

@ofalvai ofalvai requested a review from tothszabi February 16, 2022 14:30
@ofalvai ofalvai changed the title [WIP] Detect unsupported tools in projects Detect unsupported tools in projects Feb 16, 2022
@ofalvai ofalvai merged commit ee1c4fb into master Feb 16, 2022
@ofalvai ofalvai deleted the STEP-1803-unknown-tools branch February 16, 2022 15:05
@ofalvai ofalvai mentioned this pull request Feb 17, 2022
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