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

Add flag to skip inserting notes into the database #42

Merged
merged 2 commits into from
Apr 29, 2021
Merged

Conversation

ecamacho
Copy link
Contributor

In some cases, a log can contain thousands of Notes. For instance, if it has a lot of modules and is running SwiftLint as a PostBuild Phase's script. Inserting this data to the Database, can make it grow exponentially.

This PR adds an option: --skipNotes false that can be passed to the XCMetrics command so all those notes won't be inserted.

@@ -0,0 +1,56 @@
// Copyright (c) 2020 Spotify AB.
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 created a new Shared module between the client and the backend to held the models used by both. At the moment, it only contains this one. But I plan to refactor in the future to move more things here and avoid code duplication.

Copy link
Contributor

@polac24 polac24 left a comment

Choose a reason for hiding this comment

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

Looks good!

/// Don't process the Notes found in the log
/// In some cases, we can have thousands of these that can make database grow exponentially
/// while providing little value
public let skipNotes: Bool?
Copy link
Contributor

Choose a reason for hiding this comment

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

backward compatible 👍

….swift

Co-authored-by: Bartosz Polaczyk <polac24@gmail.com>
@ecamacho ecamacho merged commit 14a5d12 into main Apr 29, 2021
@ecamacho ecamacho deleted the skip_notes branch April 29, 2021 18:38
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