Don't allow to define reserved flags via Compiler.define #8690
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
|
But hxnodejs doesn't define any reserved flags? |
|
|
|
But it's not being defined yet (HaxeFoundation/hxnodejs#59). That's |
|
Oh, right. Then it's safe to merge ) |
|
Still can't merge this becaus of this: Lines 37 to 39 in f9733e7 |
|
Ultimately the problem is that documentation generation conflates We can hack around this problem first by moving that |
tobil4sk
added a commit
to tobil4sk/haxec
that referenced
this pull request
Sep 11, 2021
- Error for `-D haxe-ver` solved more generally - Add temporary function to be removed for HaxeFoundation#8690 - Minor refactoring
tobil4sk
added a commit
to tobil4sk/haxec
that referenced
this pull request
Sep 11, 2021
- Error for `-D haxe-ver` solved more generally - Add temporary function to be removed for HaxeFoundation#8690 - Minor refactoring
RealyUniqueName
pushed a commit
that referenced
this pull request
Oct 12, 2021
* Fix inconsistency between - and _ in define.json * Remove unnecessary dash conversion define.json now already provides them with dashes so this manual conversion is no longer necessary. * Move `source-header` default value setting * Split function with bool parameter * Fix missing error when doing `-D haxe-ver` * Use underscore defines internally Move all define conversions to interfaces, and internally a flag is represented using the underscore version only. This prevents duplicate defines appearing in a bunch of places, and now there are no longer tiny differences in the effects of `-D flag-a` and `-D flag_a`. Same is true for macro functions for accessing defines. * Use existing functions for accessing defines * Remove duplicate definition of the `flash` define This is already done for every target in `Common.init_platform`. * Update documentation for `net-ver` compiler flag * Remove `dump_context` made redundant in bef481f * Clean up define outputs All internal outputs of defines are now in alphabetical order * Add tests for define flag conversions * Add tests for `-D no-deprecation-warnings` * Add tests for `-D source-header` * Test for duplicate defines in `hlc.json` & `Options.txt` * [hl] Fix `-D source-header` not working with hlc (#10376) * Add tests for `source-header` on hlc * Clean up - Error for `-D haxe-ver` solved more generally - Add temporary function to be removed for #8690 - Minor refactoring * Fix `--help-defines`
|
Given all the conflicts, the best course of action here is to close. #8689 is still open to track the original issue. |
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.
Fixes #8689
Shouldn't be merged yet because it will break
hxnodejs, which defines its flags viaCompiler.define():https://github.com/HaxeFoundation/hxnodejs/blob/2587ccbffa3cc032d7ecca28d299e7357993f536/extraParams.hxml#L1-L3