Environment cleanups
#2353
Merged
Environment cleanups
#2353
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… the value is effectively a compile-time constant. Also improve the use of MARK comments so Xcode renders the intended divisions nicely.
…al behavior of its value.
…ions). Significantly simplify the implementation of the path-taking version. IMPORTANT NOTE: This is an interim step of cleanup while a much more complete revamping of this API is worked on.
gwynne
added
enhancement
|
I have a complete rework of the
(The existing API on |
…ons for the testing environment. I forgot to mention in a previous commit that support was added for `VAPOR_ENV` too...
gwynne
added
semver-minor
|
(Note: CI failures are being addressed.) |
…ary. This is necessarily a little specific to the version of SwiftPM and Xcode involved, but should at least be specific enough a check to not interfere with normal operations if the call sequence changes.
…Convertible conformance. Logger.Level is already RawRepresentable as String and that conformance can be used transparently. (And it has CaseIterable for good measure, which provides yet another way to scale this particular elevation.)
gwynne
dismissed
MrLotU’s stale review
Want a new review for new changes, dismissing old
MrLotU
approved these changes
May 11, 2020
|
Ping @tanner0101 |
tanner0101
approved these changes
May 26, 2020
gwynne
commented
May 26, 2020
gwynne
commented
May 26, 2020
tanner0101
approved these changes
May 26, 2020
|
These changes are now available in 4.6.0 |
nachoBonafonte
pushed a commit
to scope-demo/vapor
that referenced
this pull request
May 27, 2020
* New works-for-everything CI (vapor#2365) * New works-for-everything CI This version of the test workflow can be pasted without changes into any repo whose tests don't have additional dependencies and don't need to separately test that they're still Vapor-compatible (such as console-kit). Improvements over existing CI: - Runs on pushes to master as well as PRs - Full coverage of all available Swift runner images for Linux - Uses latest Xcode without hardcoding on macOS - Nice names for each test step - Nice name for the workflow itself * Apply suggestions from code review * `Environment` cleanups (vapor#2353) * Remove numerous unnecessary code stanzas. * Don't check `isRelease` in `Environment.==`, it will always be equal because the value is effectively a compile-time constant. Also improve the use of MARK comments so Xcode renders the intended divisions nicely. * Add warning to `Environment.isRelease` explaining the seemingly unusual behavior of its value. * Redo the documentation comments for `Environment.secret()` (both versions). Significantly simplify the implementation of the path-taking version. IMPORTANT NOTE: This is an interim step of cleanup while a much more complete revamping of this API is worked on. * Correctly sanitize the excess arguments Xcode passes to test invocations for the testing environment. I forgot to mention in a previous commit that support was added for `VAPOR_ENV` too... * Add sanitization of raw SwiftPM's invocation of the xctest runner binary. This is necessarily a little specific to the version of SwiftPM and Xcode involved, but should at least be specific enough a check to not interfere with normal operations if the call sequence changes. * There is no need to hardcode all the logger levels for `LosslessStringConvertible` conformance. `Logger.Level` is already `RawRepresentable` as `String` and that conformance can be used transparently. (And it has `CaseIterable` for good measure, which provides yet another way to scale this particular elevation.) Co-authored-by: Gwynne Raskind <gwynne@darkrainfall.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Clean up
Environment's implementation.Add the ability to specify the application environment using the environment variable
VAPOR_ENV(partial implementation of Support $PORT and $HOSTNAME #2333). If bothVAPOR_ENVand--envare provided, the latter always takes precedence.Environmentnow recognizes and strips the spurious arguments Xcode passes to test runners when a scheme doesn't specify any arguments of its own. It will do this especially specifically for thetestingenvironment, which is often explicitly specified toApplication's initializers instead of.detect()in unit tests. This behavior is only enabled when building in Xcode or withxcodebuild. This negates the need to explicitly override the input arguments in the non-production presets, meaning command line arguments no longer occasionally seem to vanish into thin air in certain cases.Redo the documentation comments for both versions of
Environment.secret()and significantly simplify the implementation. Important: This is an interim cleanup step pending a complete revamp of thesecret()API.Note: Pains have been taken not to change the observable behavior of any public APIs and to remain otherwise behaviorally consistent. The addition of
VAPOR_ENVcounts as new public API and thus drives this being asemver-minorchange, but in all other respects it should have no visible effect.