added export cable blacklist#592
Conversation
Dima-Lisovvenko
changed the title
Lothrazar
left a comment
Lothrazar
left a comment
There was a problem hiding this comment.
This would be a good feature to add, but it looks like the scope of the PR changes creeped outside of the scope of the feature. even if it didnt cause a merge conflicts, it looks very stressful to just read it: lots of comments removed, comments added, brackets reformatted from auto linters, lots of blank lines added, lots of blank lines removed, an arbitrary 256 threshold to limit something, theres a new withQuery() method idk even know what it does , and what does withQuery have to do with an export blacklist. Mabye some of these are fixing things that arent mentioned? smaller one off prs are better in every way.
Also doesnt help that the only text written is your "..." was there an issue request you could link to? Compared to this PR #593 which is very clear exactly what it does, and why, and what issue it fixes.
I know its been forever but on the off chance you feel like a do over i will leave this PR open, we can always close it later. it is a feature that might get revisited later, especially if people are asking for it on the tracker. For now i need to move forward to releasing 1.20 fixes and then porting to 1.21, and then going beyond
2 participants
...