Skip to content

Conversation

@dabrahams
Copy link
Contributor

No description provided.

@dabrahams
Copy link
Contributor Author

@swift-ci Please smoke test and merge

@swift-ci swift-ci merged commit 5d034a2 into master May 13, 2017
@palimondo
Copy link
Contributor

palimondo commented May 13, 2017

@dabrahams It looks like this commit contains manual edit to benchmark/CMakeLists.txt and benchmark/utils/main.swift that are not coupled with corresponding edits of benchmark/CMakeLists.txt.gyb and benchmark/utils/main.swift.gyb.

@gottesmm, how come the validation-test/benchmarks/generate-harness.test-sh we added in #8641 didn't catch this? I thought test and merge does run the validation, doesn't it?

@palimondo
Copy link
Contributor

Judging from the title of this PR, I question if the addition of Substring.swift and corresponding mods to test harness were intended here...

@dabrahams
Copy link
Contributor Author

They were intended, but they weren't supposed to be part of the same PR.

@dabrahams
Copy link
Contributor Author

AFAICT there is no edit required in those gyb files. I re-ran generate-test-harness.py, and IIUC it made the edits to main.swift and CMakeLists.txt by discovering the new tests in single-source/Substring.swift

@palimondo
Copy link
Contributor

Haha, you are quite right! Mea culpa. I forgot that additions are auto-discovered during harness generation. Editing gybs is required only when we change the template itself.

Lesson learned: don't comment as a first thing in a morning when all you've got are 10 minutes and you need to run before you can verify the code. I'm sorry Dave!

@dabrahams
Copy link
Contributor Author

dabrahams commented May 14, 2017

@palimondo n/p. The worst part was thinking I had messed up somehow.

@jrose-apple jrose-apple deleted the stdlib-coding-convention branch November 29, 2017 22:36
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.

4 participants