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

Reject resolve calls returning nil #10

Merged
merged 1 commit into from
Jun 9, 2019
Merged

Reject resolve calls returning nil #10

merged 1 commit into from
Jun 9, 2019

Conversation

vemv
Copy link
Contributor

@vemv vemv commented Jun 5, 2019

Brief

Fixes #9

QA plan

Have a look at the new tests

Author checklist

  • I have QAed the functionality
  • The PR has a reasonably reviewable size and a meaningful commit history
  • I have run the branch formatter and observed no new/significative warnings
  • I have self-reviewed the PR prior to assignment, and its build passes
  • Specifically, I have code-reviewed iteratively the PR considering the following aspects in isolation:
    • Correctness
    • Robustness (red paths, failure handling etc)
    • Modular design
    • Test coverage
    • Spec coverage
    • Documentation
    • Security
    • Performance

Reviewer checklist

  • I have checked out this branch and reviewed it locally, running it
  • I have QAed the functionality
  • I have code-reviewed iteratively the PR considering the following aspects in isolation:
    • Business-facing correctness
    • Robustness (red paths, failure handling etc)
    • Modular design
    • Test coverage
    • Spec coverage
    • Documentation
    • Security
    • Performance

@vemv vemv requested a review from thumbnail June 5, 2019 03:58
@vemv
Copy link
Contributor Author

vemv commented Jun 5, 2019

The tests (old and new) are not very fine-grained, would be happy to improve them after https://github.com/nedap/utils.spec/pull/55

@thumbnail
Copy link
Member

The tests (old and new) are not very fine-grained, would be happy to improve them after nedap/utils.spec#55

Let's do that in a different pr

@thumbnail thumbnail mentioned this pull request Jun 3, 2019
@vemv vemv merged commit dcb3f39 into master Jun 9, 2019
@vemv vemv deleted the 9 branch June 9, 2019 00:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Observe the ns's alias map, fail-fast on spurious aliases
2 participants