Skip to content
This repository has been archived by the owner on Feb 14, 2023. It is now read-only.

Run check_manifest #32

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

maithuvenkatesh
Copy link

Checking the result of adding check-manifest to the the CI pipeline.

Copy link
Collaborator

@jnothman jnothman left a comment

Choose a reason for hiding this comment

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

check_manifest: command not found

Please install it first.

Also move to after_success to avoid any dependency contamination
@jnothman jnothman changed the title TEST PR Run check_manifest Nov 21, 2019
@jnothman
Copy link
Collaborator

Seems we're running it in the wrong directory...

@thomasjpfan
Copy link
Collaborator

This does not pass locally for me with:

lists of files in version control and sdist do not match!
missing from VCS:

which includes all the generated c files from Cython and generated py files used for deprecation.

@jnothman
Copy link
Collaborator

Is there a way to make "missing from VCS" non-fatal?

@thomasjpfan
Copy link
Collaborator

Not possible yet: mgedmin/check-manifest#58

We can try work around this by dynamically getting the files that are generated, which will be passed into --ignore.

@ogrisel
Copy link
Contributor

ogrisel commented Nov 9, 2020

We might still want to make it work and adapt it to the new CD infra in https://github.com/scikit-learn/scikit-learn/blob/master/.github/workflows/wheels.yml.

@thomasjpfan
Copy link
Collaborator

As a reference: There is a check-manfiest github action that runs daily on the main repo: https://github.com/scikit-learn/scikit-learn/blob/master/.github/workflows/check-manifest.yml

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants