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

Cleanup CMakeList.txt #7647

Merged
merged 4 commits into from
Jul 3, 2019
Merged

Cleanup CMakeList.txt #7647

merged 4 commits into from
Jul 3, 2019

Conversation

niboshi
Copy link
Member

@niboshi niboshi commented Jun 29, 2019

Organized the structure of CMakeList.txt

@niboshi niboshi added the cat:code-fix Code refactoring that does not change the behavior. label Jun 29, 2019
take-cheeze
take-cheeze approved these changes Jul 2, 2019
View reviewed changes
Copy link
Member

@take-cheeze take-cheeze left a comment

Choose a reason for hiding this comment

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

Seems to be working fine on my env 👍
Will run test.

take-cheeze
take-cheeze requested changes Jul 2, 2019
View reviewed changes
Copy link
Member

@take-cheeze take-cheeze left a comment

Choose a reason for hiding this comment

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

Sorry forgot to check the conflict.
Please resolve conflict for merging

@take-cheeze
Copy link
Member

Anyway Jenkins, test this please!

@chainer-ci
Copy link
Member

Jenkins CI test (for commit b7555b5, target branch master) failed with status FAILURE.

@niboshi
Copy link
Member Author

niboshi commented Jul 2, 2019

PTAL

@take-cheeze
Copy link
Member

Jenkins, test this please!

@chainer-ci
Copy link
Member

Jenkins CI test (for commit 1a88bf4, target branch master) failed with status FAILURE.

take-cheeze
take-cheeze requested changes Jul 3, 2019
View reviewed changes
Copy link
Member

@take-cheeze take-cheeze 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 the fix.
CI seems to working fine.
Have little comment.

@niboshi
Copy link
Member Author

niboshi commented Jul 3, 2019

BTW, the previous change to move list(APPEND CUDA_NVCC_FLAGS -std=c 14) was mistakenly amended into the merge commit. I fixed that by separating commits (now it's ac47a0d).

@take-cheeze take-cheeze added st:test-and-merge State indicating that pull request is approved by a reviewer and can be merged after CI passes. ChainerX Related to ChainerX. labels Jul 3, 2019
take-cheeze
take-cheeze approved these changes Jul 3, 2019
View reviewed changes
Copy link
Member

@take-cheeze take-cheeze left a comment

Choose a reason for hiding this comment

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

LGTM 👍
Will wait for CI

@take-cheeze
Copy link
Member

Jenkins, test this please!

@chainer-ci
Copy link
Member

Jenkins CI test (for commit 5324c95, target branch master) failed with status FAILURE.

@take-cheeze
Copy link
Member

Jenkins, test this please!

@chainer-ci
Copy link
Member

Jenkins CI test (for commit 5324c95, target branch master) failed with status FAILURE.

@take-cheeze
Copy link
Member

Test failure seems to be unrelated so merging

@take-cheeze take-cheeze merged commit 98c8b25 into chainer:master Jul 3, 2019
@niboshi niboshi deleted the cleanup-cmake branch July 3, 2019 07:41
@kmaehashi kmaehashi added this to the v7.0.0b2 milestone Jul 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@take-cheeze take-cheeze take-cheeze approved these changes

Assignees

@take-cheeze take-cheeze

Labels
cat:code-fix Code refactoring that does not change the behavior. ChainerX Related to ChainerX. st:test-and-merge State indicating that pull request is approved by a reviewer and can be merged after CI passes.
Projects
None yet
Milestone
v7.0.0b2
Development

Successfully merging this pull request may close these issues.
4 participants
@niboshi @take-cheeze @chainer-ci @kmaehashi