Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
Description
Suggestion
This is a followup to#8821 (comment)
In#8821, I was renaming a rule that has other active pull requests (#8688 and#8673). No matter which PR is merged first, the other PRs will have conflicts (that git probably will not detect as conflicts).
To prevent these semantic merge conflicts/drift, I had originally tried to bring the original and renamed rules into common code, to ensure runtime parity, and just fix up the metadata details when exporting the rule object for each file (see original commit:12182f5). (I also did a similar approach for the test code).
This has one major upside (runtime parity), but several major downsides:
- more complicated and tedious code
- the resulting code circumvents some internal lint rules that ordinarily run on the test cases
- it's more tedious to introduceintentional drift if need be
We ended up deciding to go for the copypasta approach, and we'll just have to pay close attention to ensure that the changes to the rule get duplicated to the renamed version.
This got me, thinking though - is there a way we can be clever and have the ease of copypasta but also some CI-validated insurance against drift?
My thought was that wecould consider making a CI-validated diff snapshot between the relevant files, so that drift would need to be explicitly accepted by updating said snapshot.
This issue is to consider poking around and see if we can lay down a pattern for rule renames that have some CI/PR-validated drift prevention (via common code source or diff drift prevention). Another rule that this would apply to isno-throw-literal
/only-throw-error
. Maybe there are others, too.
The consensus may well also be that this is more effort to even think about than it's worth, no worries if so, we can just close it 😄
Very primitive sketch of the concept for the diff idea (but that would need significant refinement to make usable):
import{execSync}from'child_process';import*aspathfrom'path';describe('Renamed rules should have stable diffs compared to their original versions',()=>{it('(no-unnecessary-template-literals -> no-unnecessary-template-expression) rule files',()=>{letdiffString:string|undefined;try{execSync(`git diff --raw --no-index "${path.join(__dirname,'../src/rules/no-useless-template-expression.ts',)}" "${path.join(__dirname,'../src/rules/no-useless-template-literals.ts',)}"`,);// eslint-disable-next-line @typescript-eslint/no-explicit-any}catch(error:any){diffString=error.stdout.toString();}if(diffString==null){thrownewError();}expect(diffString).toMatchSnapshot();});it('(no-unnecessary-template-literals -> no-unnecessary-template-expression) test files',()=>{letdiffString:string|undefined;try{execSync(`git diff --no-index "${path.join(__dirname,'rules/no-useless-template-expression.test.ts',)}" "${path.join(__dirname,'rules/no-useless-template-literals.test.ts',)}"`,);// eslint-disable-next-line @typescript-eslint/no-explicit-any}catch(error:any){diffString=error.stdout.toString();}if(diffString==null){thrownewError();}expect(diffString).toMatchSnapshot();});});