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

Add docs build job #3157

Merged
merged 45 commits into from
Feb 28, 2023
Merged

Add docs build job #3157

merged 45 commits into from
Feb 28, 2023

Conversation

AyodeAwe
Copy link
Contributor

@AyodeAwe AyodeAwe commented Jan 18, 2023

The PR adds a docs_build process to the PR and Build workflows for this repository. The generated docs are synced to s3 for only the build workflows.

@AyodeAwe AyodeAwe added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jan 18, 2023
@AyodeAwe AyodeAwe force-pushed the add-docs-build branch 6 times, most recently from 9230a15 to ec65d20 Compare January 18, 2023 21:03
@codecov-commenter
Copy link

codecov-commenter commented Jan 18, 2023

Codecov Report

Base: 55.30% // Head: 55.30% // No change to project coverage 👍

Coverage data is based on head (9e332c9) compared to base (068629e).
Patch has no changes to coverable lines.

Additional details and impacted files
@@              Coverage Diff              @@
##           branch-23.02    #3157   +/-   ##
=============================================
  Coverage         55.30%   55.30%           
=============================================
  Files               148      148           
  Lines              9573     9573           
=============================================
  Hits               5294     5294           
  Misses             4279     4279           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ajschmidt8 ajschmidt8 changed the title add docs_build step Add docs build job Jan 19, 2023
@BradReesWork BradReesWork added this to the 23.02 milestone Jan 23, 2023
@AyodeAwe AyodeAwe marked this pull request as ready for review January 24, 2023 19:18
@AyodeAwe AyodeAwe requested a review from a team as a code owner January 24, 2023 19:18
@AyodeAwe AyodeAwe marked this pull request as draft January 24, 2023 19:19
@AyodeAwe AyodeAwe marked this pull request as ready for review January 25, 2023 15:17
@BradReesWork
Copy link
Member

@AyodeAwe there is a merge conflict

BradReesWork
BradReesWork previously approved these changes Jan 27, 2023
@ajschmidt8 ajschmidt8 requested a review from a team as a code owner February 17, 2023 23:27
ci/build_docs.sh Outdated Show resolved Hide resolved
ci/build_docs.sh Outdated Show resolved Hide resolved
dependencies.yaml Outdated Show resolved Hide resolved
@AyodeAwe
Copy link
Contributor Author

AyodeAwe commented Feb 22, 2023

@VibhuJawa how should this style check failure be handled? The error showed up after we imported your patch:

python/cugraph-dgl/cugraph_dgl/dataloading/dataloader.py:144:38: F821 undefined name 'create_tensorized_dataset'

https://github.com/rapidsai/cugraph/actions/runs/4247522209/jobs/7385666170

cc: @ajschmidt8

@VibhuJawa
Copy link
Member

VibhuJawa commented Feb 22, 2023

@VibhuJawa how should this style check failure be handled? The error showed up after we imported your patch:

python/cugraph-dgl/cugraph_dgl/dataloading/dataloader.py:144:38: F821 undefined name 'create_tensorized_dataset'

https://github.com/rapidsai/cugraph/actions/runs/4247522209/jobs/7385666170

cc: @ajschmidt8

This line create_tensorized_dataset should have been dgl.dataloading.create_tensorized_dataset like in this patch .

I think we had a bad merge which erased it. Anyways, add it and you should be good to go.

Copy link
Contributor

@rlratzel rlratzel left a comment

Choose a reason for hiding this comment

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

I see that @VibhuJawa looked over some of the non-docs related changes to cugraph-dgl files, so I'm assuming those are supposed to be in this PR.

@ajschmidt8
Copy link
Member

I see that @VibhuJawa looked over some of the non-docs related changes to cugraph-dgl files, so I'm assuming those are supposed to be in this PR.

Yes. Good call out, @rlratzel. There were some import issues that required a patch from Vibhu which we included.

Copy link
Member

@VibhuJawa VibhuJawa left a comment

Choose a reason for hiding this comment

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

LGTM

@VibhuJawa
Copy link
Member

@rlratzel , Yup those changes allow building cugraph-dgl in a environment without dgl and pytorch . The changes look good to me. Approved the PR too.

@AyodeAwe
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 8539b88 into rapidsai:branch-23.04 Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants