Skip to content

Conversation

@iSazonov
Copy link
Collaborator

Prepare to pass meta test #3504.

First commit convert tab indentations to spaces in *.resx files.
Second commit add Newlines at EOF.

Start time: {0:yyyyMMddHHmmss}
Username : {1}\{2}
Machine : {3} ({4})
Machinen : {3} ({4})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Machinen [](start = 0, length = 8)

extra 'n'?

Copy link
Collaborator Author

@iSazonov iSazonov Apr 26, 2017

Choose a reason for hiding this comment

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

Fixed.

@mirichmo mirichmo self-assigned this Apr 27, 2017
<value>Html can only be combined with Html Text format.</value>
</data>
</root>
</root>
Copy link
Member

Choose a reason for hiding this comment

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

Missing newline here

<value>One or more headers were not specified. Default names starting with "H" have been used in place of any missing headers.</value>
</data>
</root>
</root>
Copy link
Member

Choose a reason for hiding this comment

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

And here

<value>Cannot process the command because -Configuration requires an argument that is a remote endpoint configuration name. Specify this argument and try again.</value>
</data>
</root>
</root>
Copy link
Member

Choose a reason for hiding this comment

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

Here

<value>Loading personal and system profiles took {0}ms.</value>
</data>
</root>
</root>
Copy link
Member

Choose a reason for hiding this comment

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

A guess: I think your script was fairly consistent with these. If it made a change in the file, it didn't add an extra newline at the end. I'm going to stop commenting on each individual one. Please make it consistent throughout if you are going to make this change.

@mirichmo
Copy link
Member

@lzybkr - I remember that you had some thoughts on this topic so I wanted to make sure you are aware of this PR.

@lzybkr
Copy link
Contributor

lzybkr commented Apr 27, 2017

Well, my thoughts are basically - style/formatting changes shouldn't be piecemeal - we should deal with all the files at once. And I'd prefer to see one commit per "kind" of change, say replace tabs, or fix file encoding, or add/remove newlines.

The newline at the end is an annoying issue - some tools expect a newline on the last line (like wc and git), whereas a human reader, a blank line at the end needlessly takes up space on the screen, so I naturally delete it.

So my point here is - unless we have a pre-commit hook, it's a battle we can't win.

@iSazonov
Copy link
Collaborator Author

I prepared Add Metatests #3504 It cannot be activated until all the files are cleaned up.
The same applies to pre-commit hooks - if today we active it each contributor would have to make many changes to the formatings in addition to his code changes.

I believe we must first do the global file cleanup then activate the checks, first, for the current commits, then globally in the nightly tests.

We have already done a few steps:
@lzybkr removed trailing whitespace #3001 (not in *.resx).
I converted tab indentations to spaces in *.cs files #3551.

In the PR I convert tab indentations in *.resx.
Next I plan to convert tab indentations *.test.ps1 (but I can add the commit to the PR if you want) and then remove trailing whitespace in *.resx.

And in the end I plan to add Newline at EOF in all files.

I did not follow the rule "all the files at once" because @TravisEz13 asked to do a work in smaller chunks to simplify a review.

Can we follow this roadmap?

@TravisEz13
Copy link
Member

The risk of doing everything in one large change is that you won't be able to ever merge the change due to merge conflicts.

@iSazonov
Copy link
Collaborator Author

Conflict was resolved.
Newlines will be fixed in another PR.
Can we continue the code review?

@iSazonov
Copy link
Collaborator Author

iSazonov commented Jun 1, 2017

@mirichmo Could we continue with the PR?

@mirichmo mirichmo merged commit f04d2fd into PowerShell:master Jun 7, 2017
@iSazonov iSazonov deleted the resx-tabs-to-spaces branch June 19, 2017 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants