Refactor lint api #395
Refactor lint api #395
Conversation
| if Sys.file_exists dir then | ||
| get_files dir |> List.map check_file |> List.concat | ||
| else ( | ||
| print_endline | ||
| @@ Printf.sprintf | ||
| "Skipped check_package_dir since package dir %s doesn't exist" dir; | ||
| []) |
There was a problem hiding this comment.
Under what conditions is it valid for there to not be a package dir when trying to lint an opam package for publication?
There was a problem hiding this comment.
The publication tools first generate the opam metadata (OpamFile.OPAM.t) objects, and I was trying to integrate the lints at that stage, rather than after they create the required files.
punchagan
force-pushed
the
refactor-lint-api
branch
2 times, most recently
from
af73ede
to
f42e234
Compare
punchagan
force-pushed
the
refactor-lint-api
branch
from
f42e234
to
f481e5b
Compare
We download the package source repository to run lint checks on dune constraints. If the package source is already available, avoiding downloading them again can significantly speed-up the lint checks.
punchagan
force-pushed
the
refactor-lint-api
branch
from
f481e5b
to
9420d9e
Compare
Instead of downloading packages during a specific check, we change the API to accept paths to the package source directory. Also, the API now accepts parsed OPAM files instead of parsing it as part of the lint.
We sometimes want to run the lint before creating the package directory. In those cases, we skip the test to `check_package_dir`. This is helpful for the package publication tools to run lints.
Also remove get_packages from the public API
The API now makes the flag newly_published a bool option type. When the flag is None, the library automatically tries to detect if the package is a newly published one based on whether the repository has any other versions of the package already in it.
punchagan
force-pushed
the
refactor-lint-api
branch
from
9420d9e
to
7301eaf
Compare
|
The inference of "newly published" or not isn't yet exposed on the CLI, and we always need to explicitly specify that from the CLI, as of now. But, I think the rest of the API change is self contained and useful to merge even without the CLI improvement. |
I pushed a WIP commit to expose the newly-published inference and also to allow specifying the pkg_src_dirs from the CLI. I've not yet changed the usage of the CLI in the CI to use this CLI. This needs to be done once we are happy with the interface. |
punchagan
force-pushed
the
refactor-lint-api
branch
from
f05caf2
to
698bd64
Compare
punchagan
force-pushed
the
refactor-lint-api
branch
from
698bd64
to
121e4c1
Compare
This PR is a preparatory refactor to separate out the Lint functionality from the project as a separate library for use with package publication tools to prevent badly linted PRs being opened on opam-repository.
The big API change is to use parsed OPAM.t objects instead of parsing the opam files in the lint library. This should make it easier to use the library API from the publication tools. This also allows us to run the lint before creating the package directory in the
opam-repository. (In such cases, we skip thecheck_package_dirlint).Also, the download of package sources in the
check_dune_constraintstest has been removed and the downloads are now moved to themainmodule. This allows us to prevent any package downloads as part of the library, and allows us to pass the path of the directory when using the API.We also add some light-weight analysis to be able to infer if a package being linted is a "newly-published" package, or an update to an existing one. This makes the public API simpler. But, we also allow explicitly specifying this information to bypass the inference, if required. This can be used from the CLI by the CI.
The PR has a few minor clean-ups/improvements:
repo_dirargument asopam_repo_dirfor clarity