search

DefinitelyTyped / dt-mergebot

The bot which handles auto-merging your PRs
https://devblogs.microsoft.com/typescript/changes-to-how-we-manage-definitelytyped/
MIT License
112 stars 44 forks source link

Use old header parser when package.json doesn't look new #482

Closed jakebailey closed 1 year ago

jakebailey commented 1 year ago

https://github.com/DefinitelyTyped/dt-mergebot/commit/6b659eb7177af948e7e649bb65266bd16d7ef7ea#diff-2b507820ea5da67eac1be3f3c554e982520499201e3b667d69cea285a5ed59d0 and the recent changes are making the bot unhappy when detecting owner changes.

Instead of trying to figure out how to find the real merge commit and to the comparison, we can instead just pull the header code back in and use it conditionally. This will work better for older PRs which still contain index.d.ts and so won't misparse as empty owners.

~Rerunning tests doesn't seem to change anything. I had to recreate fixtures to actually get something to show my fix so I'm not sure what's going on there. I guess I need npm run update-all-fixtures?~

I reverted the tests to before the DT change and then reran tests here.

This diff is whack; https://github.com/DefinitelyTyped/dt-mergebot/compare/e49a4e17d1c5440f7e5fc37a0365ca5a305faa33...jakebailey:dt-mergebot:fix-diffing shows the difference between this PR and the change before the DT change, which should be much more reasonable.

jakebailey commented 1 year ago

I don't think we should remove this for a long time, people may undraft old PRs and we shouldn't choke on them.