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 file weight option #70

Closed
wants to merge 2 commits into from
Closed

Conversation

HerrDerb
Copy link

@HerrDerb HerrDerb commented Apr 12, 2024

Solves #69

Currently the size of a PR is determined by the amount of changes (Deletion+ Modification)

In my perception, I take on on a PR which has 120 changes in a single file much easier, as on a PR which has 120 changes across 30 files. In other words, the complexity is more interesting as the size only.

By taking a file weight into account (1 file = x changes) the labels would effectively show the complexity of the pr.

src/github.sh Outdated
@@ -5,14 +5,16 @@ GITHUB_API_HEADER="Accept: application/vnd.github.v3+json"
github::calculate_total_modifications() {
local -r pr_number="${1}"
local -r files_to_ignore="${2}"
local -r changed_file_weight="${3:-0}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

existing pattern seems to be to set the default values in the actions.yml file rather than inline here


echo $((additions + deletions))
echo $((additions + deletions + (changed_files * changed_file_weight)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

To use an example, if there's a 10 file change and the changed_file_weight is set to 10, then we're adding an additional 100 lines to the calculated diff? Is that the intention?

If you're trying to calculate the complexity of the change, that might require understanding the number of line changes per total lines in all files. For example, a 100 line file having 30 lines changes would be complexity 30.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, this change missing the else block here when files_to_ignore is set.

@HerrDerb HerrDerb force-pushed the include-file-weight branch from ccca251 to a2b9cdb Compare May 1, 2024 15:21
@HerrDerb HerrDerb requested a review from johnlk May 1, 2024 15:21
@HerrDerb
Copy link
Author

Close as outdated and not significant

@HerrDerb HerrDerb closed this Dec 13, 2024
@HerrDerb HerrDerb deleted the include-file-weight branch December 13, 2024 12:03
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