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.

Sign up for GitHub

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

Jump to bottom

Updates Build Instructions to Adjust for Devcontainer Instructions and Remove Stale Information #1179

Merged
merged 12 commits into from
Jun 20, 2023
Merged

Updates Build Instructions to Adjust for Devcontainer Instructions and Remove Stale Information #1179

merged 12 commits into from
Jun 20, 2023

Conversation

isVoid
Copy link
Contributor

@isVoid isVoid commented Jun 6, 2023

Description

closes #1174

Minor improvement:

  • updates build development environment to point to devcontainers, not rapids-compose.
  • Move point-linestring python test to correct location.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@isVoid isVoid requested a review from a team as a code owner June 6, 2023 16:23
@github-actions github-actions bot added the Python Related to Python code label Jun 6, 2023
@isVoid isVoid added tech debt Related to improving software quality non-breaking Non-breaking change Python Related to Python code improvement Improvement / enhancement to an existing function and removed Python Related to Python code labels Jun 6, 2023
jarmak-nv
jarmak-nv approved these changes Jun 6, 2023
View reviewed changes
Copy link
Contributor

@jarmak-nv jarmak-nv left a comment

Choose a reason for hiding this comment

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

Some capitalization/wording updates and proposed changes. Approving, and leaving it up to you which suggestions you want to accept.

docs/source/developer_guide/build.md Outdated Show resolved Hide resolved
docs/source/developer_guide/build.md Outdated Show resolved Hide resolved
docs/source/developer_guide/build.md Outdated Show resolved Hide resolved
docs/source/developer_guide/build.md Outdated Show resolved Hide resolved
docs/source/developer_guide/build.md Outdated Show resolved Hide resolved
docs/source/developer_guide/development_environment.md Outdated Show resolved Hide resolved
docs/source/developer_guide/development_environment.md Outdated Show resolved Hide resolved
docs/source/developer_guide/development_environment.md Outdated Show resolved Hide resolved
harrism
harrism requested changes Jun 6, 2023
View reviewed changes
Copy link
Member

@harrism harrism left a comment

Choose a reason for hiding this comment

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

Thanks for improving these!

docs/source/developer_guide/build.md Outdated Show resolved Hide resolved
docs/source/developer_guide/build.md Show resolved Hide resolved
docs/source/developer_guide/build.md Outdated Show resolved Hide resolved
docs/source/developer_guide/development_environment.md Show resolved Hide resolved
docs/source/developer_guide/development_environment.md Show resolved Hide resolved
docs/source/developer_guide/build.md Show resolved Hide resolved
docs/source/developer_guide/build.md Outdated Show resolved Hide resolved
isVoid and others added 5 commits June 7, 2023 10:49
@isVoid @jarmak-nv
Apply documentation updates
b0a7ca0 
Co-authored-by: Ben Jarmak <104460670+jarmak-nv@users.noreply.github.com>
@isVoid @harrism
Apply suggestions from code review
2415223 
Co-authored-by: Mark Harris <783069+harrism@users.noreply.github.com>
@isVoid
Update build and test instructions
8f9ba16
@isVoid
Add reference to dependency file generator
b30b236
@isVoid
typo
5c4a037
@isVoid isVoid requested a review from harrism June 7, 2023 15:30
harrism
harrism approved these changes Jun 7, 2023
View reviewed changes
Copy link
Member

@harrism harrism left a comment

Choose a reason for hiding this comment

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

Looks good. One more small discrepancy in build location depending on dev container vs build.sh

Maybe we can make it so these two build approaches put the builds in the same place?!

docs/source/developer_guide/build.md Outdated Show resolved Hide resolved
docs/source/developer_guide/build.md Outdated Show resolved Hide resolved
@isVoid
Copy link
Contributor Author

isVoid commented Jun 12, 2023

Maybe we can make it so these two build approaches put the builds in the same place?!

cc @trxcllnt Is this possible?

@harrism
Copy link
Member

harrism commented Jun 13, 2023

It's definitely possible. However it would make our build.sh different from other RAPIDS repos (but in a good way?).

@isVoid
Copy link
Contributor Author

isVoid commented Jun 20, 2023

It's definitely possible. However it would make our build.sh different from other RAPIDS repos (but in a good way?).

I'm going to start an offline chat on this topic. Meanwhile, I think we can go ahead and merge this PR.

@isVoid
Copy link
Contributor Author

isVoid commented Jun 20, 2023

/merge

@rapids-bot rapids-bot bot merged commit 9d91cd3 into rapidsai:branch-23.08 Jun 20, 2023
@isVoid isVoid changed the title Updates Build Instructions to Adjust for Devcontainer Instructions and Remove Stale Infomation Updates Build Instructions to Adjust for Devcontainer Instructions and Remove Stale Information Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@harrism harrism harrism approved these changes

@jarmak-nv jarmak-nv jarmak-nv approved these changes

@trxcllnt trxcllnt Awaiting requested review from trxcllnt trxcllnt is a code owner automatically assigned from rapidsai/cuspatial-python-codeowners

@thomcom thomcom Awaiting requested review from thomcom thomcom is a code owner automatically assigned from rapidsai/cuspatial-python-codeowners

Assignees
No one assigned
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python Related to Python code tech debt Related to improving software quality
Projects
cuSpatial
Status: Todo
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[DOC]: Build Instruction Contains Information about a Removed Module
3 participants
@isVoid @harrism @jarmak-nv