-
Notifications
You must be signed in to change notification settings - Fork 148
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
xWebAdministration: Resolve style guideline violations for hashtables #524
Comments
Thought I would take a crack at this, but the first one it has problems with is line 2 here: $script:handlers = @{
'aspq-Integrated-4.0' = (New-Object PSObject -Property @{
Name = 'aspq-Integrated-4.0';
Path = '*.aspq';
Verb = 'GET,HEAD,POST,DEBUG';
Type = 'System.Web.HttpForbiddenHandler';
PreCondition = 'integratedMode,runtimeVersionv4.0'
});
### A lot more similar keys
} How is that supposed to be formatted to comply with the rule? Seems silly to have to load it into a variable first? |
Let me check on that tonight. It looks like it formatted correctly except the semi-colon that shouldn’t be necessary. This is a new rule so it could be edge cases that we haven’t thought of. It is possible it is a false negative. In regards to adding it to a script scope variable, not sure why it is done this way. It might be able to be improved as well but could be another issue/PR. Let me check on that as well. |
Sounds good! I wasn't really referring to the script scope variable, but the only solution I found that was accepted by Measure-Hashtable was doing something like $property = @{
Name = 'aspq-Integrated-4.0';
Path = '*.aspq';
Verb = 'GET,HEAD,POST,DEBUG';
Type = 'System.Web.HttpForbiddenHandler';
PreCondition = 'integratedMode,runtimeVersionv4.0'
}
$script:handlers = @{
'aspq-Integrated-4.0' = (New-Object PSObject -Property $property);
### A lot more similar keys
} I apparently felt that was obvious to everyone when I wrote the comment late last night 😂 |
Ah, then I follow you :) Didn’t realize that you tested different ways of solving it :) I don’t think we should need to do such workaround. @SSvilen do you have a thought on this? Is this and edge case we need to handle in then rule? Is it hashtable inside an another hashtable that is the problem here? 🤔 |
@tomlarse Thank you for finding this and actually manually test against the new rule! 😃 |
In the rule I've assumed that the hashtable ends with \s*} (as regex). '});' would violate it, } won't |
Seems like it was the indentation it didn't like. I'll remove the semicolons and put the paranthesis on it's own line then? @johlju? |
I'm not sure what the parenthesis contribute with? It looks much nicer without them :) $script:handlers = @{
'aspq-Integrated-4.0' = New-Object PSObject -Property @{
Name = 'aspq-Integrated-4.0';
Path = '*.aspq';
Verb = 'GET,HEAD,POST,DEBUG';
Type = 'System.Web.HttpForbiddenHandler';
PreCondition = 'integratedMode,runtimeVersionv4.0'
}
### more similar keys
} $script:handlers = @{
'aspq-Integrated-4.0' = (New-Object PSObject -Property @{
Name = 'aspq-Integrated-4.0';
Path = '*.aspq';
Verb = 'GET,HEAD,POST,DEBUG';
Type = 'System.Web.HttpForbiddenHandler';
PreCondition = 'integratedMode,runtimeVersionv4.0'
}
)
### more similar keys
} |
@tomlarse Yes we should not need the parenthesis in that call. Also remove the semi-colon. Thanks! 😃 |
@SSvilen Thank you! 🙂 I missed the indentation when I looked at the code snippet, I wrongly assumed it was indented correctly 😄 |
- Changes to xWebAdministration - Updated hashtables in the repo to adhere to the style guidelines described at https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#correct-format-for-hashtables-or-objects (issue #524).
- Changes to xWebAdministration - Updated hashtables in the repo to adhere to the style guidelines described at https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#correct-format-for-hashtables-or-objects (issue dsccommunity#524).
We are adding a PSSA rule that validates hashtables to be correct according to the style guideline (PR PowerShell/DscResource.Tests#348).
Since we have opt-in to the common test that validates these rules we need to make sure to update all the hashtables that still does not comply to the style guideline. After that we will merge the rule into the test framework.
Below are all the style guideline violations that need to be resolved for this repo.
The text was updated successfully, but these errors were encountered: