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

remove docs support in build.sh, remove test_wheel.sh, other small cleanup #60

Merged
merged 2 commits into from
Oct 28, 2024

Conversation

jameslamb
Copy link
Member

@jameslamb jameslamb commented Oct 21, 2024

Pulling more changes off of #53, to make that diff smaller and get us closer to having full CI running in this repo.

  • removes docs argument for build.sh (docs building is handled by ci/build_docs.sh)
  • removes ci/test_wheel.sh
  • removes unnecessary wheel_smoke_test_cugraph.py (copied over from cugraph but not needed here, to be replaced by other scripts you can see in the diff of Add CI for cuGraph-GNN #53)
  • other minor cleanup of CI scripts

@jameslamb jameslamb added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Oct 21, 2024
@jameslamb jameslamb requested review from a team as code owners October 21, 2024 16:27
@jameslamb jameslamb requested a review from bdice October 21, 2024 16:27
@jameslamb jameslamb changed the title remove docs support in build.sh, other small cleanup remove docs support in build.sh, remove test_wheel.sh, other small cleanup Oct 21, 2024
@@ -1,40 +0,0 @@
#!/bin/bash
Copy link
Member Author

Choose a reason for hiding this comment

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

@alexbarghi-nv this looks like it's not used on #53.

I strongly suspect the version here was adopted from https://github.com/rapidsai/cugraph/blob/branch-24.12/ci/test_wheel.sh, which in the cugraph repo is never used with cugraph-dgl / cugraph-pyg: https://github.com/search?q=repo%3Arapidsai%2Fcugraph%20%22test_wheel.sh%22&type=code

I strongly suspect test_wheel.sh could just be removed, and that the ci/wheel_smoke_test_${package_name}.py scripts being proposed on #53 are unnecessary, and covering functionality already covered by the ci/test_wheel_${package_name}.sh scripts.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, this can be taken out

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! I've just pushed changes to #53 removing the smoke-testing stuff that was added there.

@jameslamb
Copy link
Member Author

@alexbarghi-nv am I right about this PR (that we want to remove docs-building details in this repo)? If so could you approve? Then I can go get a ci-codeowners / packaging-codeowners approval and we can merge this.

@alexbarghi-nv
Copy link
Member

@alexbarghi-nv am I right about this PR (that we want to remove docs-building details in this repo)? If so could you approve? Then I can go get a ci-codeowners / packaging-codeowners approval and we can merge this.

We're moving docs into a new repo and not building them here, so that's correct.

@bdice
Copy link
Contributor

bdice commented Oct 28, 2024

Changes look good, thanks for the clarifying questions above.

@alexbarghi-nv alexbarghi-nv merged commit 25d1b55 into rapidsai:branch-24.12 Oct 28, 2024
48 checks passed
@jameslamb jameslamb deleted the remove-docs branch October 28, 2024 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improves an existing functionality non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants