microsoft / pxt-microbit

A Blocks / JavaScript code editor for the micro:bit built on Microsoft MakeCode
https://makecode.microbit.org
Other
726 stars 638 forks source link

Default dependencies include Radio which is unnecessary and frustating #4838

Closed keble6 closed 1 year ago

keble6 commented 2 years ago

Describe the bug When saving Makecode to GitHub the default dependencies list includes Radio. This leads to a non-obvious problem if you later try to add Bluetooth.

To Reproduce Create code which doesn't use Bluetooth, save to Github

Expected behavior The pxt.json will not include a Radio dependency

Screenshots See pxt.json in https://keble6.github.io/t/

micro:bit version (please complete the following information): not hardware related

Desktop (please complete the following information):

abchatra commented 2 years ago

@keble6 can you please elaborate non obvious issues?

keble6 commented 2 years ago

When trying to add Bluetooth Makecode can then remove other extensions before adding Bluetooth

abchatra commented 2 years ago

Students who use Radio are magnitude more than Bluetooth. Hence we made it default. Unfortunately they are not compatible and hence the need to remove the other. This is intended behavior.

keble6 commented 2 years ago

Sorry, I should explain further: I created a makecode project called bug-demo and daved to github here

Note that it does not use either Radio or Bluetooth.

If you look at pxt.json in Github you see the following:

"dependencies": { "core": "", "radio": "", "microphone": "*" } If you now try to add the Bluetooth extension you get a "Some extensions will be removed" warning. If you agree then it can happen that some code blocks are removed which were not Radio blocks. I think there is something in that removal process that can really mess up your code.

bsiever commented 2 years ago

radio vs. bluetooth is distinct from other extensions because they are mutually exclusive and radio is a default. In other cases a developer explicitly includes extensions that their new extension either may need itself or may want to pass on to users.

I think the challenge is identifying when radio should be a dependency and when it shouldn't. One option --- radio would be included as a dependency either: a) if/when a radio block is used in the code and b) when the developer explicitly opts-in (covers extensions that want to ensure radio is always present for users even if the extension doesn't use it)...and omitted otherwise. I still don't think that's entirely obvious.

Perhaps the GUI pxt.json view for extension developers could include a 3-way option: Require radio, Require bluetooth, Allow either. Ideally it would update to "Require radio" if the extension itself directly used any radio blocks.

keble6 commented 2 years ago

Agreed, good proposal.