-
Notifications
You must be signed in to change notification settings - Fork 16
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
Detect file moves in local source tracking #574
Conversation
…g into ew/file-moves
src/shared/localShadowRepo.ts
Outdated
const message = 'Files were found that have the same basename and hash. Skipping the commit of these files'; | ||
this.logger.warn(message); | ||
await Lifecycle.getInstance().emitWarning(message); | ||
await Lifecycle.getInstance().emitTelemetry({ eventName: 'moveFileHashBasenameCollisionsDetected' }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we're capturing telemetry anyway, let's capture the filenames
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we're capturing telemetry anyway, let's capture the filenames.
and always Promise.all unrelated consecutive awaits
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "ignore" Maps will not contain all of the filenames since we are "setting" over the top of the new values as we go. We could add some more logic to calculate them, but i don't know if it is worth the extra complexity. Telemetry will let us know how often this happens
QA notes: preview looks good before a deploy. retrieve preview actual deploy create new packageDir and move a file to the same dir over there (other-dir/classes/FileUtilities.cls and its cls.-meta.xml) move GeoCodingService files the same way, with
📓 the seems like a lot of getStatus being called. At least they're fast. renaming an existing file [should still show up as a add + delete] moving a field from 1 custom object to another (test case from @VivekMChawla)
move an unresolvable file (README.md moved from force-app/main/default/objects/ to other-dir/classes/ move an entire bundle
✅ moving a whole LWC gives you the expected results and logs
move customLabels file deploy preview where we move a file and it's changed in the org [expect a conflict in the new location]
|
refactor: registry from STL
QA notes: ✅ copy paste is treated as an add |
@@ -75,5 +79,7 @@ describe('it detects image file moves ', () => { | |||
expect(await repo.getChangedFilenames()) | |||
.to.be.an('array') | |||
.with.lengthOf(0); | |||
|
|||
delete process.env.SF_BETA_TRACK_FILE_MOVES; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I have a slight preference for doing these in an before|afterEach
so that they next test added doesn't have to remember to unset it.
QA: |
What does this PR do?
when a file is moved from one location to another within a project (ex: from
force-app/main/default/classes
toother-dir/classes
) but has the same filename and content, it is NOT considered an add and a delete. Source tracking updates its local filesystem tracking to "clear" those changes.This does not apply if you move files between different parents (ex: moving a field from
Foo__c
toBar__c
) or moving a file from one LWC to another--that still counts as an add a delete.What issues does this PR fix or reference?
forcedotcom/cli#2682
@W-15354803@