This repository has been archived by the owner on Apr 14, 2023. It is now read-only.
Overhaul WEC subscription formatting #4
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
There's only very minor differences between the two sets and they don't appear to be related to legacy operating system support like XP? Remove them in the absence of a good reason to keep them (NT6/* appears newer).
- Made all the XPath queries much more readable via an improved layout - Fix formatting inconsistencies (tabs to spaces, indentation, etc...) - Added missing XML declaration - Added missing PublisherName XML element
ralish
force-pushed
the
wec-sub-improvs
branch
from
14c0a9b
to
75eb91d
Compare
|
We'll take a look at the changes. We'll likely accept the majority of these changes. Thanks. |
|
I like the reorg suggestions, but I will say that I appreciate the "Targets:" information in the Description field of each subscription. |
|
We are going to revisit this since we need to update this for Windows 10. I think we will generally end up accepting most of the pull request. We just haven't decided what we are going to keep versus not keep and the main author hasn't been available to help make that determination. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This PR may be more substantial than desired and so you choose to decline this, but that's fine, I'm only submitting these PRs in case they're of interest upstream. The commit history should be fairly clear, but in short, there's zero functional changes. Instead, the sample WEC subscriptions have had their formatting substantially overhauled to be IMO far more readable and maintainable. There's particular emphasis on the embedded
XPathqueries which now have consistent formatting with splitting over multiple lines and appropriate indentation. This makes parsing and editing them far easier, not to mention spotting errors which may not necessarily be syntactic (as in PR #2).In addition, I've renamed the subscription files to have more accurate & verbose file names (I don't see why they need to be short) and removed all the
samples/*subscriptions. I'm unclear what their purpose is and a diff between them and theNT6/*subscriptions shows only very minor changes. TheNT6/*samples appear to be newer, and the changes don't suggest they're for supporting downlevel OSs, so in the absence of a compelling reason to keep them they've been removed.Comments welcome.