Skip to content
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

Rename Copy.Item.Tests.ps1 to match naming convention #10701

Merged
merged 1 commit into from
Jun 12, 2020

Conversation

romero126
Copy link
Contributor

PR Summary

Rename Copy.Item.Tests.ps1 -> Copy-Item.Tests.ps1
in order to match naming convention inside \test\powershell\Modules\Microsoft.PowerShell.Management

PR Context

All filenames in \test\powershell\Modules\Microsoft.PowerShell.Management use Verb-Command.Tests when naming their files. This one however does not match that standard.

PR Checklist

@iSazonov
Copy link
Collaborator

iSazonov commented Oct 7, 2019

We have many files not matching naming convention.
If we want have a consistency we must fix everything or fix nothing.

@romero126
Copy link
Contributor Author

I agree completely. However we cant just fix everything all at once.. That's why we need the approach fix it when you see it. :)

@vexx32
Copy link
Collaborator

vexx32 commented Oct 7, 2019

Mm true. I could see fixing the naming on each file in a given folder, doing it one folder at a time, perhaps?

@iSazonov
Copy link
Collaborator

iSazonov commented Oct 7, 2019

I don't see a value from the change because we use modern IDEs with power and fast search, c# plugins allows us to follow by references, also sometimes we have many classes in one file - as result file name lost value but in the same time renaming complicates commit history.

@TravisEz13 TravisEz13 added this to the 7.1.0-preview.1 milestone Nov 23, 2019
@ghost ghost assigned daxian-dbw Nov 23, 2019
@daxian-dbw
Copy link
Member

... in the same time renaming complicates commit history.

I kinda agree with @iSazonov on that. Renaming a file will cause the file history to be "lost" (or to be more accurate, harder to retrieve). So if it's not necessary, maybe we should avoid renaming files.

@vexx32
Copy link
Collaborator

vexx32 commented Dec 10, 2019

Is it so frequent that we need to dig up file history?

The additional time needed to dig up file history is usually not immense, and not needed very often. I would be surprised if that's necessary anywhere near as much as folx need to go looking for the file. Time lost finding the file as it is now v.s. time lost finding the file as it was before the rename weighted depending on frequency of need.

I would argue that having a consistent naming convention will make maintenance easier both for the PS team and folx in the community, especially folx just starting to look at contributing. In my opinion the consistency is valuable here, but y'all may have different ideas. 🙂

@iSazonov
Copy link
Collaborator

Is it so frequent that we need to dig up file history?

If you want to get better understanding how a code works you will look history frequently.

@ghost ghost added the Review - Needed The PR is being reviewed label May 27, 2020
@ghost
Copy link

ghost commented May 27, 2020

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Mainainer, Please provide feedback and/or mark it as Waiting on Author

Copy link
Member

@daxian-dbw daxian-dbw left a comment

Choose a reason for hiding this comment

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

Conflicts resolved.
@romero126 Sorry that this PR has been overlooked for so long ...
The general guideline is to avoid renaming files to keep the history handy. But in this particular case, the history is not very interesting. So I decide to take this change.

@daxian-dbw
Copy link
Member

The change in this PR has nothing to do with the CI failure, so I will force merge it.

@ghost ghost removed the Review - Needed The PR is being reviewed label Jun 12, 2020
@daxian-dbw daxian-dbw merged commit 8d85c14 into PowerShell:master Jun 12, 2020
@iSazonov iSazonov added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Jun 12, 2020
@ghost
Copy link

ghost commented Jun 25, 2020

🎉v7.1.0-preview.4 has been released which incorporates this pull request.:tada:

Handy links:

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.

None yet

5 participants