Skip to content

Improve count with intent to add to CI #281

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

Merged
merged 61 commits into from
Jan 19, 2024
Merged

Conversation

sbryngelson
Copy link
Member

Improve ./mfc.sh count with intent to add to CI. Removes comment lines from count but includes openacc directives.

@sbryngelson
Copy link
Member Author

sbryngelson commented Dec 29, 2023

Really want this to print the line count different between master and current.

Example https://github.com/tinygrad/tinygrad/actions/runs/7306212421/workflow?pr=2886

@henryleberre
Copy link
Member

Really want this to print the line count different between master and current.

Example https://github.com/tinygrad/tinygrad/actions/runs/7306212421/workflow?pr=2886

Do you have a use-case in mind?

@sbryngelson
Copy link
Member Author

Really want this to print the line count different between master and current.
Example tinygrad/tinygrad/actions/runs/7306212421/workflow?pr=2886

Do you have a use-case in mind?

I would like to know how many lines are being added or subtracted via a PR. I see some PRs where it's not immediately clear that a refactor actually reduces line count. The GitHub PR linecount diff includes all sorts of things that we don't care about.

@henryleberre
Copy link
Member

Really want this to print the line count different between master and current.
Example tinygrad/tinygrad/actions/runs/7306212421/workflow?pr=2886

Do you have a use-case in mind?

I would like to know how many lines are being added or subtracted via a PR. I see some PRs where it's not immediately clear that a refactor actually reduces line count. The GitHub PR linecount diff includes all sorts of things that we don't care about.

I see. For the specific case where you make a code-only refactor, you can subtract the + and - lines as shown on the PR's Github page since when you only modify a line, it counts as both a deletion and an addition. I just realized that things like adding (not changing) golden files do skew results, although they don't have many lines.

@sbryngelson
Copy link
Member Author

Really want this to print the line count different between master and current.
Example tinygrad/tinygrad/actions/runs/7306212421/workflow?pr=2886

Do you have a use-case in mind?

I would like to know how many lines are being added or subtracted via a PR. I see some PRs where it's not immediately clear that a refactor actually reduces line count. The GitHub PR linecount diff includes all sorts of things that we don't care about.

I see. For the specific case where you make a code-only refactor, you can subtract the + and - lines as shown on the PR's Github page since when you only modify a line, it counts as both a deletion and an addition. I just realized that things like adding (not changing) golden files do skew results, although they don't have many lines.

Yes this mostly does the job. Though I figure the CI mechanisms used for this would also be useful for other presumably more important things.

@sbryngelson sbryngelson marked this pull request as ready for review January 16, 2024 00:22
@sbryngelson
Copy link
Member Author

@henryleberre I think this is the best this PR is going to get without more serious effort. The commit-bot is a no-go as you noticed. I was thinking that one could add an argument to ./mfc.sh count that allows a diff between two versions of MFC/src/* but this seems like more work/tricks than I can muster at the moment (I don't quite understand how it's going through and making those nice line count tables to begin with).

@sbryngelson sbryngelson marked this pull request as draft January 19, 2024 01:40
@sbryngelson sbryngelson marked this pull request as ready for review January 19, 2024 03:53
@sbryngelson sbryngelson merged commit 0b05ff3 into master Jan 19, 2024
@sbryngelson sbryngelson deleted the sbryngelson-patch-1 branch January 19, 2024 03:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants