Skip to content

Conversation

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Apr 18, 2019

PR Summary

Fix formatting issues in SMA with CodeFormatter.

Please review commit by commit.

PR Context

PR Checklist

@iSazonov iSazonov added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Apr 18, 2019
@iSazonov iSazonov added this to the 7.0.0-preview.1 milestone Apr 18, 2019
var isHidden = cacheEntry.member.GetCustomAttributes(typeof(HiddenAttribute), false).Any();
return new PSProperty(cacheEntry.member.Name, this, obj, cacheEntry) { IsHidden = isHidden } as T;
}
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems odd to have the brace indented particularly since the brace isn't even needed

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

// if it points to a directory, add the basename back onto the path...
qualifiedPath = Path.Combine(qualifiedPath, Path.GetFileName(fileBaseName));
}
string qualifiedPath = Path.Combine(path, fileBaseName);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems to make the code harder to read since it's optimizing for the non-Unix case

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

moduleSpecification.Name,
rootedPath, // fileBaseName
/*extension*/null,
/*extension*/null,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this shouldn't be indented so far. would be better to have the comment at the end like the others or even better change this to use named params

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

res.Add(new PSTypeName(typeConstraint.TypeName));
typeConstraintAdded = true;
}
if (!typeConstraintAdded)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here as well, overly indented

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

case AttributeAst attributeAst:
{
}
PSTypeNameAttribute attribute = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same, other cases below

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

// associated with this powershell
// data structure handler object to handle all
// communications with the client
// that is associated with this
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably reformat this comment altogether to just a few lines

Copy link
Collaborator Author

@iSazonov iSazonov Apr 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All comments in the code block was reformatted.

{
if (policy == ExecutionPolicy.Undefined)
executionPolicy = null;
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overly indented

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

properties = new[] { stringConstant.Value };
break;
}
{
Copy link
Member

@SteveL-MSFT SteveL-MSFT Apr 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should just remove the braces in whole file

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whole file was fixed.

@iSazonov iSazonov self-assigned this Apr 24, 2019
@iSazonov iSazonov merged commit ceed73d into PowerShell:master Apr 24, 2019
@iSazonov iSazonov deleted the codeformatter-sma-1 branch April 24, 2019 14:27
TravisEz13 added a commit that referenced this pull request Apr 27, 2019
* return correct casing of filesystem path during normalization

* handle UNC case

* handle case to add trailing separator back

* add support for NTFS streams syntax

* add case-sensitive test

* only apply correct casing to directories

* handle 8.3 path syntax

* fix short path processing

* Remove elements which do not work on earlier version on Windows PowerShell (#9411)

* Use `Environment.NewLine` for new lines in `ConsoleHost` code (#9392)

* Run CodeFormatter for Eventing (#9394)

* Run CodeFormatter for MarkdownRender (#9398)

* Run CodeFormatter for Security module (#9399)

* Run CodeFormatter for WSMan.Runtime (#9401)

* Run CodeFormatter for WSMan.Management (#9400)

* Allow CI to run on branches with this name pattern: feature* (#9415)

* Run CodeFormatter with BraceNewLine,UsingLocation,FormatDocument,NewLineAbove rules (#9393)

* Build(deps): Bump NJsonSchema from 9.13.30 to 9.13.34 (#9421)

* Build(deps): Bump NJsonSchema from 9.13.34 to 9.13.35 (#9429)

Bumps [NJsonSchema](https://github.com/rsuter/NJsonSchema) from 9.13.34 to 9.13.35.
- [Release notes](https://github.com/rsuter/NJsonSchema/releases)
- [Commits](https://github.com/rsuter/NJsonSchema/commits)

Signed-off-by: dependabot[bot] <support@dependabot.com>

* Build(deps): Bump Newtonsoft.Json (#9431)

Bumps [Newtonsoft.Json](https://github.com/JamesNK/Newtonsoft.Json) from 12.0.1 to 12.0.2.
- [Release notes](https://github.com/JamesNK/Newtonsoft.Json/releases)
- [Commits](JamesNK/Newtonsoft.Json@12.0.1...12.0.2)

Signed-off-by: dependabot[bot] <support@dependabot.com>

* Build(deps): Bump Newtonsoft.Json from 12.0.1 to 12.0.2 (#9434)

Bumps [Newtonsoft.Json](https://github.com/JamesNK/Newtonsoft.Json) from 12.0.1 to 12.0.2.
- [Release notes](https://github.com/JamesNK/Newtonsoft.Json/releases)
- [Commits](JamesNK/Newtonsoft.Json@12.0.1...12.0.2)

Signed-off-by: dependabot[bot] <support@dependabot.com>

* Add Preview assets for msix (#9375)

* Adding tests for `Remove-Module` (#9276)

* Build(deps): Bump gulp from 4.0.0 to 4.0.1 in /test/common/markdown (#9441)

* Move warning message to EndProcessing so it only shows up once (#9385)

* Run Codeformatter for SMA (#9402)

* Fix the failed test and update 'Publish-TestResults' to make AzDO fail the task when any tests failed (#9457)

* Enable building on Kali Linux (#9471)

* Build test packages for windows, linux-x64, linux-arm, linux-arm64 and macOS (#9476)

* Build(deps): Bump NJsonSchema from 9.13.35 to 9.13.36 (#9478)

* Update Release_Process.md

Add item for global tool


Co-authored-by: Steve Lee <slee@microsoft.com>
Co-authored-by: Steve Lee (POWERSHELL) <slee@ntdev.microsoft.com>
Co-authored-by: Aditya Patwardhan <adityap@microsoft.com>
Co-authored-by: Ilya <darpa@yandex.ru>
Co-authored-by: Dongbo Wang <dongbow@microsoft.com>
Co-authored-by: null <dependabot[bot]@users.noreply.github.com>
Co-authored-by: pougetat <thomas.pouget-abadie@ensimag.grenoble-inp.fr>
Co-authored-by: Andrew <anmenaga@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants