-
Notifications
You must be signed in to change notification settings - Fork 42
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
#230 Create metrics summary files #379
#230 Create metrics summary files #379
Conversation
@lueFlake looks very good, but where is the test? How do you know that it works? |
@yegor256 I tested it in a local environment, working on tests rn |
@yegor256 added tests, task is basically done i'd like to add more flexible summarization (for now every summary contains only a certain number of aggregations). By "flexible" i mean adding new directory (smth like "summary_aggregations/") in the root where i put scripts which aggregate metrics in different ways (i call them "aggregations" but i mean things like sum, average, mean of a metric). |
Also here is an example of script working on cam-2021-07-08.zip, it's a part of LOC.csv |
tests/steps/test-summarize.sh
Outdated
touch "${TARGET}/temp/repos-to-aggregate.txt" | ||
echo "repo1" >> "${TARGET}/temp/repos-to-aggregate.txt" | ||
mkdir -p "${dir1}" | ||
echo "50" > "${dir1}/file1.m.LOC" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lueFlake try to give this file a more complicated name, for example:
echo "50" > "${dir1}/file1 -p (test ) \n?.m.LOC"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
tests/steps/test-summarize.sh
Outdated
dir1="${TARGET}/measurements/repo1" | ||
mkdir -p "${TARGET}/temp" | ||
touch "${TARGET}/temp/repos-to-aggregate.txt" | ||
echo "repo1" >> "${TARGET}/temp/repos-to-aggregate.txt" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lueFlake would be better to define a variable $repo
and then use it everywhere, instead of the repo1
literal. Also, try to give it a more complicated name, like a.x/b-one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
tests/steps/test-summarize.sh
Outdated
echo "repo1" >> "${TARGET}/temp/repos-to-aggregate.txt" | ||
echo "repo2" >> "${TARGET}/temp/repos-to-aggregate.txt" | ||
mkdir -p "${dir1}" | ||
echo "50" > "${dir1}/file1.m.LOC" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lueFlake same here, the file name is too simple, it doesn't really help us reveal possible errors in the script
@lueFlake looks great! see a few comments above |
@yegor256 can you review the code? I resolved your comments |
@lueFlake thanks! |
As in #230, added
steps/summarize.sh
step which generates csv reports in 'data/summary/{metric}.csv'. Used this structure of csv:repository,count,sum,average,mean,min,max