MinkExtension icon indicating copy to clipboard operation
MinkExtension copied to clipboard

A suggestion

Open jorgealonso78 opened this issue 9 years ago • 6 comments

jorgealonso78 avatar Nov 11 '16 20:11 jorgealonso78

Are you sure these are the changes you're suggesting in #259? To me it just looks like a refactoring when now 1 method is handling all url visiting stuff.

aik099 avatar Nov 11 '16 22:11 aik099

yes that was my suggestion

jorgealonso78 avatar Nov 12 '16 00:11 jorgealonso78

Ah, then A suggestion PR/issue title was super non-helpful in understanding it.

aik099 avatar Nov 12 '16 08:11 aik099

@aik099 Haha! I was very surprised to see your

This better to be sent as a Pull Request right away.

However, I would like this shortcut to see in the next release, not in master.

spolischook avatar Nov 12 '16 08:11 spolischook

This better to be sent as a Pull Request right away.

@spolischook , I've asked to send a PR, because this is a code change and it better can be reviewed this way. That had nothing to do with code itself 😉

aik099 avatar Nov 12 '16 08:11 aik099

Removing the method is a BC break, because users could be calling it programmatically. So I'm -1 on this change (there is not enough benefit to counterbalance the BC break)

stof avatar Nov 12 '16 09:11 stof