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

Should exclude Lock files like Cargo.lock #206

Closed
futurist opened this issue Nov 18, 2020 · 14 comments
Closed

Should exclude Lock files like Cargo.lock #206

futurist opened this issue Nov 18, 2020 · 14 comments
Labels
enhancement New feature or request

Comments

@futurist
Copy link

Describe the bug
What count in rust project, the Cargo.lock file also counted.

Like: https://github.com/cgag/loc

To Reproduce

  1. In Any cargo based rust project, run scc
  2. The result show Cargo Lock lines.

Expected behavior
Cargo.lock is auto generated file, should not be counted.

Desktop (please complete the following information):

  • OS: macOS 10.15.5
  • scc version 2.13.0
@boyter
Copy link
Owner

boyter commented Nov 18, 2020

This is one I am on the fence about. I actually have it set to count cargo.lock as a filetype which might exist in a project. Because even though its auto-generated its part of the project.

It actually follows the same logic that say rg or ag would, because they both would look through the file,

$ rg 58fb5e95d83b38284460a5fda7d6470aa0b8844d283a0b614b8535e880800d2d
Cargo.lock
264:"checksum aho-corasick 0.7.6 (registry+https://github.com/rust-lang/crates.io-index)" = "58fb5e95d83b38284460a5fda7d6470aa0b8844d283a0b614b8535e880800d2d"

Generally I suggest using a .ignore file if you would not want to count this file.

Although I am prepared to be convinced otherwise.

@futurist
Copy link
Author

futurist commented Nov 19, 2020

It's different software as rg or ag, they do search and scc do count, count people's works. If Cargo.lock can be count, then is it right that many other machine generated code, like yarn.lock, pubspec.lock, Podfile.lock, package-lock.json...

ag will search it mostly because this file sometimes will be pushed(for CI/CD etc.), and sometimes should be ignored:

https://dev.to/gajus/stop-using-package-lock-json-or-yarn-lock-3ddi

The origin of this misuse is NPM documentation. It should instead explain that package-lock.json should only be committed to the source code version control when the project is not a dependency of other projects, i.e. package-lock.json should only by committed to source code version control for top-level projects (programs consumed by the end user, not other programs).

And most of lock file also has below lines likewise:

yarn.lock
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.

@boyter boyter closed this as completed in 3fd994e Dec 15, 2020
@boyter
Copy link
Owner

boyter commented Dec 15, 2020

Convinced. Pushed into master. Closing down.

lukas-brenning added a commit to lukas-brenning/scc that referenced this issue Jan 13, 2021
* Update README.md

* SLOCCount came before cloc

* resolve boyter#206

* First cut at boyter#210

* Add "gohtml" and "gotxt" to the list of extensions for Go templates

These are the default extensions that e.g. GoLand uses, and some other
tools as well I believe. It's useful as it disambiguates between
templates for html/template and text/template.

* partial fix for boyter#214

* resolve after feedback

* Mostly ready for boyter#211

* real fix for issue214

* few tests and typo

* Resolvehttps://github.com/boyter/issues/208

* Update to remove wrong Go version mentioned boyter#220

Co-authored-by: Ben Boyter <[email protected]>
Co-authored-by: AlDanial <[email protected]>
Co-authored-by: Ben Boyter <[email protected]>
Co-authored-by: Martin Tournoij <[email protected]>
@LinusU
Copy link

LinusU commented Sep 21, 2021

@boyter would it be possible to also exclude package-lock.json out of the box? For the same reason as as above

@boyter
Copy link
Owner

boyter commented Sep 21, 2021

Hmm that is just a JSON file though. If you need this now, add an exception in .ignore and that should take care of it.

Have you got a sample of what it has in the header? Might be able to use that to exclude.

@LinusU
Copy link

LinusU commented Sep 22, 2021

I looked at that but there wasn't any useful header I think:

{
  "name": "name",
  "version": "0.0.0",
  "lockfileVersion": 1,
  "requires": true,
  "dependencies": {
    ...
  }
}

I'm currently passing -M package-lock.json to work around it, but I would love it if this specific file would be ignored by default ☺️

@itay-grudev
Copy link

@boyter the package-lock.json is a for all practical purposes equivalent to Cargo.lock. The naming is now industry standard and can be safely ignored.

Have you got a sample of what it has in the header?

It's a JSON - doesn't really have a header, but here are some common fields that you can test if they exist at the root level:

{
  "name": "",
  "lockfileVersion": 3,
  "requires": true,
  "packages": {}
}

For a tiny 500 line project the package-lock.json adds 7000 lines of code and $200,000 to the estimation to an otherwise $9k project.

@boyter
Copy link
Owner

boyter commented Jul 5, 2023

I guess we could add something similar to the scc --exclude-dir argument and populate it with some of these cases. Starting with package-lock.json.

How does scc --excluded-file sound for this?

@boyter boyter reopened this Jul 5, 2023
@boyter boyter added the enhancement New feature or request label Jul 5, 2023
@itay-grudev
Copy link

itay-grudev commented Jul 6, 2023

@boyter Yes - to both. We could really use such an argument, but also I strongly believe that generated files like package-lock.json should be excluded by default, as counting them just doesn't provide useful information. In fact it skews total estimations in the wrong direction.

@itay-grudev
Copy link

A more convention-over-configuration approach with sane defaults would work best.

@NathanielHill
Copy link

@boyter would second here that for JS/TS projects package-lock.json is functionally equivalent to Cargo.lock so should be excluded by default for consistency with previous decision.

@NathanielHill
Copy link

Awesome tool btw! For extra context, it adds 50ksloc and $2 million to my project estimate. It's not hard to add to .ignore or -M package-lock.json (because I don't want to commit a .ignore for a single line)... but would be nice if it just worked for TS/JS based projects

@boyter
Copy link
Owner

boyter commented Mar 11, 2024

Ill ensure this gets into the next release.

@boyter boyter moved this to In Progress in Release 3.3.0 Apr 30, 2024
@boyter
Copy link
Owner

boyter commented Apr 30, 2024

Done. Ignore files, gitignore files and the following package-lock.json,Cargo.lock,yarn.lock,pubspec.lock,Podfile.lock are now ignored by default in the output.

@boyter boyter closed this as completed Apr 30, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Release 3.3.0 Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
No open projects
Status: Done
Development

No branches or pull requests

5 participants