Skip to content

[validation-test] Add line-directive doctests #810

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

Merged

Conversation

modocache
Copy link
Contributor

What's in this pull request?

This adds the standard Python code header to utils/line-directive. It also adds that script's tests to the validation-test targets.

Why merge this pull request?

  • It makes the script more consistent with the rest of the Python code in the Swift codebase.
  • It adds the proper copyright to the file.
  • It ensures the tests for this file pass, without requiring developers to run them manually.

What downsides are there to merging this pull request?

  • There's only a single doctest in utils/line-directive. It's not a very robust test suite in the first place, so the value of adding it to validation-test is questionable.
  • One could argue that a code header should be added to all Python files in the codebase at once, in a single commit. I'm not doing so here because I happened upon this file when using it, and decided to send a quick pull request, instead of trying to find all the remaining Python in this repository.

Add code headers missing from `utils/line-directive`, as per the template
from swiftlang#762.
`utils/line-directive` is used when building Swift (see
`cmake/modules/AddSwift.cmake`), and includes some doctests. In order
to make sure it continues to behave as intended, add its tests to
`validation-test`.
gribozavr added a commit that referenced this pull request Dec 29, 2015
[validation-test] Add line-directive doctests
@gribozavr gribozavr merged commit 8338233 into swiftlang:master Dec 29, 2015
@gribozavr
Copy link
Contributor

@modocache Thank you!

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.

2 participants