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

#230 Create metrics summary files #379

Merged
merged 18 commits into from
Oct 16, 2024

Conversation

lueFlake
Copy link
Contributor

@lueFlake lueFlake commented Oct 9, 2024

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

@yegor256
Copy link
Owner

yegor256 commented Oct 9, 2024

@lueFlake looks very good, but where is the test? How do you know that it works?

@lueFlake
Copy link
Contributor Author

lueFlake commented Oct 9, 2024

@yegor256 I tested it in a local environment, working on tests rn

@lueFlake
Copy link
Contributor Author

@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).

@lueFlake
Copy link
Contributor Author

lueFlake commented Oct 15, 2024

Also here is an example of script working on cam-2021-07-08.zip, it's a part of LOC.csv
{3D14201D-7A79-432B-A12B-0613400E9362}

steps/summarize.sh Outdated Show resolved Hide resolved
touch "${TARGET}/temp/repos-to-aggregate.txt"
echo "repo1" >> "${TARGET}/temp/repos-to-aggregate.txt"
mkdir -p "${dir1}"
echo "50" > "${dir1}/file1.m.LOC"
Copy link
Owner

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"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved

dir1="${TARGET}/measurements/repo1"
mkdir -p "${TARGET}/temp"
touch "${TARGET}/temp/repos-to-aggregate.txt"
echo "repo1" >> "${TARGET}/temp/repos-to-aggregate.txt"
Copy link
Owner

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved

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"
Copy link
Owner

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

@yegor256
Copy link
Owner

@lueFlake looks great! see a few comments above

@lueFlake
Copy link
Contributor Author

@yegor256 can you review the code? I resolved your comments

@yegor256 yegor256 merged commit 741e861 into yegor256:master Oct 16, 2024
11 of 12 checks passed
@yegor256
Copy link
Owner

@lueFlake thanks!

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