Azure-sdk-for-js: [EngSys] Fix rush reset-workspace script

[ramya0820]

Since the pnpm.lock file is regenerated on local, during merge conflicts, it ends up overriding many of the changes if present on master.

Specifically, running rush reset-workspace rush update didn't update the versions to latest ones as we might expect.
And since merge conflicts in this file can be pretty large, things may get missed like it happened in #5983.
Deleting the file and running above fixes the issue of having stale versions.

It maybe a good idea to configure rush reset-workspace script to delete the pnpmp.lock file and have it be regenerates to avoid issues such as these. cc @kurtzeborn

[ramya0820]

Looks like this would impact all the libraries and work being pushed to master.
As I don't think there was any guideline to delete the pnpm.lock file and regenerate as such?
Since pnpmp.lock file gets checked in as part of almost every other PR, this also likely explains the intermittent browser test failures on CI as of Oct 24th.
cc @AlexGhiondea @ramya-rao-a @mikeharder

[bterlson]

We need to be intentional about updates to this file. My opinions follow 😀

PRs which don't add/remove dependencies should have no changes to the lockfile. During development, you should not be running rush update --full or deleting the lockfile.

If you need to add/remove a dependency, the only changes to the lockfile should be the added/removed dependencies. I think Rush is configured such that this happens today - it tries to keep the delta to a minimum.

Aside from that, we should probably be updating the lockfile at a regular cadence in dedicated PRs. This seems preferable to each PR possibly including lockfile updates. Each PR updating the lockfile would likely cause lots of annoying merge conflicts.

[KarishmaGhiya]

Yes I agree with @bterlson. I'll cc @bsiegel for rush reset-workspace script. I know we don't always want to delete the pnpm-lock file and want to do the minimal changes to pnpm lock file, so I don't think defaulting the behavior to deleting the lock file everytime is an option. In cases you have merge conflicts in pnpm-lock file, you can follow these steps :
1. copy pnpm-lock from master
2. rush update
3. git add .
4. git rebase --continue

[bsiegel]

So reset-workspace is meant as a workaround to the problem that git clean will screw up your workspace because of Rush's use of symlinks. So the script just removes the symlinks, runs git clean, and then restores the symlinks. It's not really meant to do anything else.

If we want to make it easier to remove/regenerate the lockfile, we should do it in a different script. However, on the topic of whether we -want- to make it easier to do that, I agree with the above.

[mikeharder]

Personally, I would just remove the reset-workspace command, and tell people to manually run rush unlink followed by git clean.

[KarishmaGhiya]

Closing this issue, as there is no action to be taken.