Skip to content

Conversation

@oiu850714
Copy link
Contributor

@oiu850714 oiu850714 commented Mar 22, 2022

I tried to add some test cases for SimpleArray

Copy link
Member

@yungyuc yungyuc left a comment

Choose a reason for hiding this comment

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

It will make the merge message more descriptive if branch name and the title of the PR to be more specific.

@yungyuc yungyuc added the test testing and continuous integration label Mar 22, 2022
@oiu850714 oiu850714 changed the title WIP: Try to add tests Add tests for SimpleArray Mar 22, 2022
@oiu850714 oiu850714 deleted the branch solvcon:master March 22, 2022 07:31
@oiu850714 oiu850714 closed this Mar 22, 2022
@oiu850714 oiu850714 deleted the master branch March 22, 2022 07:31
@oiu850714 oiu850714 restored the master branch March 22, 2022 07:36
@oiu850714
Copy link
Contributor Author

oiu850714 commented Mar 22, 2022

It's awkward that I accidentally closed the PR while renaming the branch...
Can I open another PR and mention this PR again?

@yungyuc
Copy link
Member

yungyuc commented Mar 22, 2022

Does github prevent you from reopening it? Let me try to do it ...

@yungyuc yungyuc reopened this Mar 22, 2022
@oiu850714
Copy link
Contributor Author

oiu850714 commented Mar 22, 2022

It will make the merge message more descriptive if branch name and the title of the PR to be more specific.

It seems that I can only change the branch I want to merge and cannot change the target branch(oiu850714/master).
Therefore I only modify the PR title and merge description.

IndexError,
r"SimpleArray::validate_shape\(\): empty index"
):
invalid_empty_idx = ()
Copy link
Member

Choose a reason for hiding this comment

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

Good test.

@yungyuc yungyuc merged commit 68681c7 into solvcon:master Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test testing and continuous integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants