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 repository results standard data table #3995

Closed
wants to merge 3 commits into from

Conversation

pstreef
Copy link
Contributor

@pstreef pstreef commented Feb 13, 2024

What's changed?

A new standard data table is added (will be created for all recipes)

What's your motivation?

The SourcesFileResults data table is too detailed on the one hand but lacks precision on the other. This data-table makes it possible to uniquely identify a recipe running on a repository and retrieve the aggregated occurrences and estimated time savings.

Anything in particular you'd like reviewers to focus on?

Naming, I had my doubts about the data-table name other canididates are:
RecipeResults
RecipeRepositoryResults

Anyone you would like to review specifically?

@jkschneider

Have you considered any alternatives or workarounds?

An alternative would be to fix the SourcesFileResults data table to have the options (or a unique way to identify which results belong to which recipe + options in a list of recipes)

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

@pstreef pstreef requested a review from jkschneider February 13, 2024 11:47
}
}

public static String serializeOption(String name, Object value) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

doubting between this more readable serialization or json (makes matching a lot easier)

Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't JSON basically be the same? Now you would get something like:

[foo="false", bar="1", baz="null", qux=`Foo"bar"`]

with JSON:

{"foo":false, "bar":1, "baz":null, "qux":"Foo\"bar\""}

Of these I think JSON looks better and it also preserves basic data types and can again be deserialized as JSON.

Of course there will be some additional escaping when serialized as CSV. But that applies to both formats.

@@ -0,0 +1,128 @@
/*
* Copyright 2022 the original author or authors.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Copyright 2022 the original author or authors.
* Copyright 2024 the original author or authors.

}
}

public static String serializeOption(String name, Object value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't JSON basically be the same? Now you would get something like:

[foo="false", bar="1", baz="null", qux=`Foo"bar"`]

with JSON:

{"foo":false, "bar":1, "baz":null, "qux":"Foo\"bar\""}

Of these I think JSON looks better and it also preserves basic data types and can again be deserialized as JSON.

Of course there will be some additional escaping when serialized as CSV. But that applies to both formats.

@@ -0,0 +1,71 @@
/*
* Copyright 2023 the original author or authors.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Copyright 2023 the original author or authors.
* Copyright 2024 the original author or authors.

@pstreef pstreef closed this Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants