-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 /lib/ files to CamelCase #303
Conversation
Hi @jianchun, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
TTYL, MSBOT; |
I definitely approve. A concern here is that it might be very difficult to merge this change into various feature branches because of the renames. I looked into this a bit and it seems like the "easiest" thing to do might be to rename all the files in a commit off the original branch point and then rebase the entire branch on top of the rename commit so that as far as git is concerned, all the changes in the branch were always with respect to the newly-named files, but it sounded pretty fragile to me, and for published branches this is not an acceptable approach. I haven't tried this yet, but it might work if you made a similar commit in each feature branch before merging master into those branches ( |
For the various Example: However since SIMD in all-CAPS is a convention we've already used in the code, I think it's reasonable to go with SIMD in these names. In all cases, when the file is named after a type, the file name should have the same casing as the type. |
Agree with |
@jianchun Thanks for pushing this forward! |
@dilijev Thanks for the info! I'll try a merge in feature branch to see how intelligent git is. @ianwjhalliday @dilijev Yes, exactly because the code used SIMD... CFG... as class names. @tfarina Thanks for filing the issue and discussion. |
@ianwjhalliday @dilijev One option is to have |
Ah the class names I see. Yes, I think renaming the files now in the desired casing is a good start and then changing the class names later if we find time. |
As for rules on acronyms in PascalCase/camelCase, I just found a handy article by Microsoft: https://msdn.microsoft.com/en-us/library/141e06ef(v=vs.71).aspx
So we should use However since |
@jianchun @ianwjhalliday Yes, I agree we should fix the filenames in one go since this is probably more traumatic than refactoring the type names later, which we can do more easily on an as-seen basis. @jianchun You're already doing this but I'll reiterate: renames should be done in a commit separately from any other changes to help Git realize the renames easier. That said, in a separate commit, might it be a good idea to add TODOs on the classes that should be renamed? |
Not sure how I feel about camel casing this particular example. "opcode" is a single-word abbreviation of "operation code". But I think we already have types that are spelled with |
@dotnet-bot test Windows x64_test please |
Do we know how Windows machines will handle this/what (if anything) people will need to do to merge their changes after this goes through? On StackOverflow, the results I'm seeing for this topic are pretty scary. |
Also, I prefer "Backend" over "BackEnd", as backend is one word. It's also less invasive (only a couple files vs. moving whole directory). |
+1 on Backend. |
Yes, we have most code spells |
SGTM |
Since there's no good way to track progress on this kind of code review I made myself a table. I'll check off items as I look.
|
Ok, renaming to |
I stand by |
I'm undecided on |
Updated checklist. I'll come back to this later. |
@dilijev Yeah, you're right, I only noticed the lib->Lib change after. Also, I think AsmJs is better, and it's how we use it in most of our codebase. |
thanks for driving this |
@dilijev Thanks for looking! It is still hard to keep track. The list was created on an older version and some comments no longer apply. |
I just performed a mock merge to BTW, Also I didn't touch |
@jianchun, awesome, thanks for verifying! Definitely puts my mind at ease. Skimmed through the rest of the list and it lgtm. |
@MikeHolman Thank you! Could you please also try to merge it to your feature branch, to double test and just in case? You can simply merge |
@dilijev I read your current list and here are some replies:
|
@MikeHolman after sleeping on it I also agree |
@jianchun Sounds good. I'll update the list to mark those items as resolved. |
Here's the filtered list of everything I haven't marked off yet:
|
@jianchun Yup, tried on my big branches and merge seems ok. Only issue I'm seeing is with my new projects still referring to "lib", which I will have to fix up (as you noted). I also tried adding a new file, since name on Windows file system "lib" didn't get renamed to "Lib". When unstaged it appears listed as untracked file "lib/asdf.txt", but once staged it turns into added file "Lib/asdf.txt", so git seems to have enough understanding and compensates appropriately. |
@MikeHolman Thanks for verification! |
@MikeHolman @jianchun looks like it will be fairly smooth |
@jianchun remaining list of concerns
|
@dilijev Thanks for looking! |
@jianchun Sounds good. Looks like most things are resolved. On the split usage, I think we should prefer The terminology and precedent also seems to be split down the middle. In this case I'm in favor of just making the minimal changes to make it work because there's no need to be more disruptive than necessary. |
We had a mix of inconsistent file name styles. This creates problems on Linux as we had to modify source code includes to the same casing as real file names. This change attempts rename (most) /lib/ files to CamelCase. ``` lib/Backend => Lib/Backend BackEnd.cpp Backend.cpp BackEnd.h Backend.h BackEndOpcodeAttrAsmJs.cpp BackendOpCodeAttrAsmJs.cpp BackEndOpcodeAttrAsmJs.h BackendOpCodeAttrAsmJs.h BackEndOpCodeList.h BackendOpCodeList.h CodegenNumberAllocator.cpp CodeGenNumberAllocator.cpp GlobOptBailout.cpp GlobOptBailOut.cpp SCCLiveness.cpp SccLiveness.cpp SCCLiveness.h SccLiveness.h lib/Backend/amd64 => Lib/Backend/amd64 MdOpcodes.h MdOpCodes.h lib/Backend/arm => Lib/Backend/arm MdOpcodes.h MdOpCodes.h lib/Backend/arm64 => Lib/Backend/arm64 MdOpcodes.h MdOpCodes.h lib/Backend/i386 => Lib/Backend/i386 MdOpcodes.h MdOpCodes.h lib/common => Lib/Common BackEndAPI.h BackendApi.h commoninl.h CommonInl.h resource.h Resource.h lib/common/common => Lib/Common/Common cfglogger.cpp CfgLogger.cpp cfglogger.h CfgLogger.h GetCurrentFrameID.h GetCurrentFrameId.h int16math.h Int16Math.h SmartFPUControl.cpp SmartFpuControl.cpp SmartFPUControl.h SmartFpuControl.h lib/common/core => Lib/Common/Core api.h Api.h AutoFILE.h AutoFile.h lib/common/DataStructures => Lib/Common/DataStructures comparer.h Comparer.h dictionary.h Dictionary.h Dlist.h DList.h growingArray.cpp GrowingArray.cpp growingArray.h GrowingArray.h interval.h Interval.h list.h List.h stack.h Stack.h lib/common/Exceptions => Lib/Common/Exceptions reporterror.cpp ReportError.cpp reporterror.h ReportError.h lib/common/Memory => Lib/Common/Memory collectionstate.h CollectionState.h heapblock.inl HeapBlock.inl heapbucket.cpp HeapBucket.cpp heapbucket.h HeapBucket.h heapbucket.inl HeapBucket.inl heapinfo.cpp HeapInfo.cpp heapinfo.h HeapInfo.h leakreport.cpp LeakReport.cpp leakreport.h LeakReport.h lib/common/util => Lib/Common/Util pinned.cpp Pinned.cpp pinned.h Pinned.h lib/jsrt => Lib/Jsrt chakracommon.h ChakraCommon.h jsrtcontext.cpp JsrtContext.cpp jsrtcontext.h JsrtContext.h jsrtExternalObject.cpp JsrtExternalObject.cpp jsrtExternalObject.h JsrtExternalObject.h jsrtinternal.h JsrtInternal.h jsrtRuntime.cpp JsrtRuntime.cpp jsrtRuntime.h JsrtRuntime.h jsrtThreadService.cpp JsrtThreadService.cpp jsrtThreadService.h JsrtThreadService.h lib/Parser => Lib/Parser alloc.cpp Alloc.cpp alloc.h Alloc.h hash.cpp Hash.cpp hash.h Hash.h hashfunc.cpp HashFunc.cpp jsscan.js JsScan.js parse.cpp Parse.cpp parse.h Parse.h regcodes.h RegCodes.h RegexOpcodes.h RegexOpCodes.h RegexRunTime.cpp RegexRuntime.cpp RegexRunTime.h RegexRuntime.h scan.cpp Scan.cpp scan.h Scan.h lib/Runtime/Base => Lib/Runtime/Base scriptcontextbase.h ScriptContextBase.h ThreadContextTLSEntry.cpp ThreadContextTlsEntry.cpp ThreadContextTLSEntry.h ThreadContextTlsEntry.h threadservicewrapper.h ThreadServiceWrapper.h lib/Runtime/ByteCode => Lib/Runtime/ByteCode AsmJSByteCodeDumper.cpp AsmJsByteCodeDumper.cpp AsmJSByteCodeDumper.h AsmJsByteCodeDumper.h BackEndOpcodeAttr.cpp BackendOpCodeAttr.cpp BackEndOpcodeAttr.h BackendOpCodeAttr.h ByteCodeAPI.h ByteCodeApi.h byteCodeCacheReleaseFileVersion.h ByteCodeCacheReleaseFileVersion.h Opcodes.cpp OpCodes.cpp lib/Runtime/Language => Lib/Runtime/Language asmjs.cpp AsmJs.cpp AsmJS.h AsmJs.h AsmJsBuiltinNames.h AsmJsBuiltInNames.h AsmJSBytecodeGenerator.cpp AsmJsByteCodeGenerator.cpp AsmJSBytecodeGenerator.h AsmJsByteCodeGenerator.h AsmJSCodeGenerator.cpp AsmJsCodeGenerator.cpp AsmJSCodeGenerator.h AsmJsCodeGenerator.h AsmJSEncoder.cpp AsmJsEncoder.cpp AsmJSEncoder.h AsmJsEncoder.h AsmJSEncoder.inl AsmJsEncoder.inl AsmJSEncoderHandler.inl AsmJsEncoderHandler.inl AsmJSJitTemplate.h AsmJsJitTemplate.h AsmJSLink.cpp AsmJsLink.cpp AsmJSLink.h AsmJsLink.h AsmJSModule.cpp AsmJsModule.cpp AsmJSModule.h AsmJsModule.h AsmJSTypes.cpp AsmJsTypes.cpp AsmJSTypes.h AsmJsTypes.h AsmJSUtils.cpp AsmJsUtils.cpp AsmJSUtils.h AsmJsUtils.h SIMDBool16x8Operation.cpp SimdBool16x8Operation.cpp SIMDBool16x8Operation.h SimdBool16x8Operation.h SIMDBool16x8OperationX86X64.cpp SimdBool16x8OperationX86X64.cpp SIMDBool32x4Operation.cpp SimdBool32x4Operation.cpp SIMDBool32x4Operation.h SimdBool32x4Operation.h SIMDBool32x4OperationX86X64.cpp SimdBool32x4OperationX86X64.cpp SIMDBool8x16Operation.cpp SimdBool8x16Operation.cpp SIMDBool8x16Operation.h SimdBool8x16Operation.h SIMDBool8x16OperationX86X64.cpp SimdBool8x16OperationX86X64.cpp SIMDFloat32x4Operation.cpp SimdFloat32x4Operation.cpp SIMDFloat32x4Operation.h SimdFloat32x4Operation.h SIMDfloat32x4OperationX86X64.cpp SimdFloat32x4OperationX86X64.cpp SIMDFloat64x2Operation.cpp SimdFloat64x2Operation.cpp SIMDFloat64x2Operation.h SimdFloat64x2Operation.h SIMDfloat64x2OperationX86X64.cpp SimdFloat64x2OperationX86X64.cpp SIMDInt16x8Operation.cpp SimdInt16x8Operation.cpp SIMDInt16x8Operation.h SimdInt16x8Operation.h SIMDInt16x8OperationX86X64.cpp SimdInt16x8OperationX86X64.cpp SIMDInt32x4Operation.cpp SimdInt32x4Operation.cpp SIMDInt32x4Operation.h SimdInt32x4Operation.h SIMDInt32x4OperationX86X64.cpp SimdInt32x4OperationX86X64.cpp SIMDInt8x16Operation.cpp SimdInt8x16Operation.cpp SIMDInt8x16Operation.h SimdInt8x16Operation.h SIMDInt8x16OperationX86X64.cpp SimdInt8x16OperationX86X64.cpp SIMDUint16x8Operation.cpp SimdUint16x8Operation.cpp SIMDUint16x8Operation.h SimdUint16x8Operation.h SIMDUint16x8OperationX86X64.cpp SimdUint16x8OperationX86X64.cpp SIMDUInt32x4Operation.cpp SimdUint32x4Operation.cpp SIMDUInt32x4Operation.h SimdUint32x4Operation.h SIMDUInt32x4OperationX86X64.cpp SimdUint32x4OperationX86X64.cpp SIMDUint8x16Operation.cpp SimdUint8x16Operation.cpp SIMDUint8x16Operation.h SimdUint8x16Operation.h SIMDUint8x16OperationX86X64.cpp SimdUint8x16OperationX86X64.cpp SIMDUtils.cpp SimdUtils.cpp SIMDUtils.h SimdUtils.h lib/Runtime/Language/amd64 => Lib/Runtime/Language/amd64 AsmJSJitTemplate.cpp AsmJsJitTemplate.cpp stackframe.cpp StackFrame.cpp stackframe.h StackFrame.h stackframe.inl StackFrame.inl lib/Runtime/Language/arm => Lib/Runtime/Language/arm stackframe.cpp StackFrame.cpp stackframe.h StackFrame.h lib/Runtime/Language/arm64 => Lib/Runtime/Language/arm64 stackframe.cpp StackFrame.cpp stackframe.h StackFrame.h lib/Runtime/Language/i386 => Lib/Runtime/Language/i386 AsmJSJitTemplate.cpp AsmJsJitTemplate.cpp stackframe.cpp StackFrame.cpp stackframe.h StackFrame.h lib/Runtime/Library => Lib/Runtime/Library dataview.cpp DataView.cpp dataview.h DataView.h JavascriptSIMDBool16x8.cpp JavascriptSimdBool16x8.cpp JavascriptSIMDBool16x8.h JavascriptSimdBool16x8.h JavascriptSIMDBool32x4.cpp JavascriptSimdBool32x4.cpp JavascriptSIMDBool32x4.h JavascriptSimdBool32x4.h JavascriptSIMDBool8x16.cpp JavascriptSimdBool8x16.cpp JavascriptSIMDBool8x16.h JavascriptSimdBool8x16.h JavascriptSIMDFloat32x4.cpp JavascriptSimdFloat32x4.cpp JavascriptSIMDFloat32x4.h JavascriptSimdFloat32x4.h JavascriptSIMDFloat64x2.cpp JavascriptSimdFloat64x2.cpp JavascriptSIMDFloat64x2.h JavascriptSimdFloat64x2.h JavascriptSIMDInt16x8.cpp JavascriptSimdInt16x8.cpp JavascriptSIMDInt16x8.h JavascriptSimdInt16x8.h JavascriptSIMDInt32x4.cpp JavascriptSimdInt32x4.cpp JavascriptSIMDInt32x4.h JavascriptSimdInt32x4.h JavascriptSIMDInt8x16.cpp JavascriptSimdInt8x16.cpp JavascriptSIMDInt8x16.h JavascriptSimdInt8x16.h JavascriptSIMDUint16x8.cpp JavascriptSimdUint16x8.cpp JavascriptSIMDUint16x8.h JavascriptSimdUint16x8.h JavascriptSIMDUInt32x4.cpp JavascriptSimdUint32x4.cpp JavascriptSIMDUInt32x4.h JavascriptSimdUint32x4.h JavascriptSIMDUint8x16.cpp JavascriptSimdUint8x16.cpp JavascriptSIMDUint8x16.h JavascriptSimdUint8x16.h javascripttypednumber.cpp JavascriptTypedNumber.cpp javascripttypednumber.h JavascriptTypedNumber.h moduleroot.cpp ModuleRoot.cpp moduleroot.h ModuleRoot.h SIMDBool16x8Lib.cpp SimdBool16x8Lib.cpp SIMDBool16x8Lib.h SimdBool16x8Lib.h SIMDBool32x4Lib.cpp SimdBool32x4Lib.cpp SIMDBool32x4Lib.h SimdBool32x4Lib.h SIMDBool8x16Lib.cpp SimdBool8x16Lib.cpp SIMDBool8x16Lib.h SimdBool8x16Lib.h SIMDFloat32x4Lib.cpp SimdFloat32x4Lib.cpp SIMDFloat32x4Lib.h SimdFloat32x4Lib.h SIMDFloat64x2Lib.cpp SimdFloat64x2Lib.cpp SIMDFloat64x2Lib.h SimdFloat64x2Lib.h SIMDInt16x8Lib.cpp SimdInt16x8Lib.cpp SIMDInt16x8Lib.h SimdInt16x8Lib.h SIMDInt32x4Lib.cpp SimdInt32x4Lib.cpp SIMDInt32x4Lib.h SimdInt32x4Lib.h SIMDInt8x16Lib.cpp SimdInt8x16Lib.cpp SIMDInt8x16Lib.h SimdInt8x16Lib.h SIMDUint16x8Lib.cpp SimdUint16x8Lib.cpp SIMDUint16x8Lib.h SimdUint16x8Lib.h SIMDUInt32x4Lib.cpp SimdUint32x4Lib.cpp SIMDUInt32x4Lib.h SimdUint32x4Lib.h SIMDUint8x16Lib.cpp SimdUint8x16Lib.cpp SIMDUint8x16Lib.h SimdUint8x16Lib.h typedArrayEnumerator.cpp TypedArrayEnumerator.cpp typedArrayEnumerator.h TypedArrayEnumerator.h lib/Runtime/Library/amd64 => Lib/Runtime/Library/amd64 javascriptfunctiona.asm JavascriptFunctionA.asm lib/Runtime/Library/InJavascript => Lib/Runtime/Library/InJavascript intl.js.bc.32b.h Intl.js.bc.32b.h intl.js.bc.64b.h Intl.js.bc.64b.h ```
LGTM. Signing off. |
@dilijev Thanks a lot! |
We had a mix of inconsistent file name styles. This creates problems on
Linux as we had to modify source code includes to the same casing as real
file names. This change attempts rename (most) /lib/ files to CamelCase.