Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V4 updates for beta testing #147

Merged
merged 62 commits into from
Mar 22, 2022
Merged

V4 updates for beta testing #147

merged 62 commits into from
Mar 22, 2022

Conversation

Amerlander
Copy link
Collaborator

A static hosted version of these changes can be seen at betamc.calliope.cc

@Amerlander
Copy link
Collaborator Author

@abchatra we updated or removed all extensions inside preferredRepos in the targetconfig.ts.

@abchatra
Copy link
Collaborator

This is not just preferredRepos. We can't serve the extensions which doesn't have MIT license (This is more of a legal issue than technical). Even approvedRepos have to be MIT license.

For example: https://github.com/calliope-edu/pxt-SCD30 doesn't have MIT license.

Is there any issue in adding the MIT license for them? All these repos are severed text files any way so users can use them as library.

@joernalraun
Copy link
Collaborator

For example: https://github.com/calliope-edu/pxt-SCD30 doesn't have MIT license.

Is there any issue in adding the MIT license for them? All these repos are severed text files any way so users can use them as library.

We just didn't know that an MIT license is necessary. The license has been updated at all the extensions.
Maybe extensions without license could be automatically excluded?

@Amerlander
Copy link
Collaborator Author

@joernalraun we only updated the preffered repos, not all listed. The pxt-SCD30 has MIT, but its only mentioned inside the Readme, there is the License file missing.

I'll go through the list and add the MIT license or remove them if they have other licenses.

@Amerlander
Copy link
Collaborator Author

@abchatra I checked now every extension and removed it if there was no license file with MIT License included.

Since pxt-calliope is a fork of pxt-microbit there are several extensions you are still using in the Microbit Target.

The extension Tinkertanker/pxt-tinkercademy-microbot has no license at all (https://github.com/microsoft/pxt-microbit/blob/b9d2eeff8116b75fa50f663091b0e3acd800b7bc/targetconfig.json#L37).

The following extensions have a MIT License mentioned in the Readme, but wthout the full license text and no license file. So they are the same like the https://github.com/calliope-edu/pxt-SCD30 you mentioned:

Annikken/pxt-Andee
LaboratoryForPlayfulComputation/pxt-BlockyTalkyBLE
PaulDFoster/pxt-microbit-GY521
Tinkertanker/microDriver_SHT2x
Tinkertanker/uDriver_PCA9585
chevyng/pxt-ucl-junkrobot
mbitfun/pxt-katakana

I removed all of them in the calliope target, but they are still present in the microbit target.

@abchatra
Copy link
Collaborator

Thanks @Amerlander We do have an issue to track down these extensions authors in microbit for the license.
microsoft/pxt-microbit#1769

@abchatra
Copy link
Collaborator

We will probably remove these extensions from microbit as well if the license is not updated. However we don't want to add additional dependencies for these now in calliope.

Thanks for doing this and sorry for the inconvenience.

@joernalraun
Copy link
Collaborator

joernalraun commented Mar 21, 2022

Thanks for doing this and sorry for the inconvenience.

Hi @abchatra is there anything else we could do now to help getting the process through?

@abchatra
Copy link
Collaborator

This is good to go. We are checking on the build failures and will merge after that.

@abchatra abchatra requested a review from riknoll March 22, 2022 11:04
@riknoll riknoll merged commit 3e0c9b4 into microsoft:master Mar 22, 2022
@riknoll
Copy link
Member

riknoll commented Mar 22, 2022

Weird, Github gave the commit a green checkmark even though the build failed. We need to investigate the yotta config for this repo... @Amerlander what is the state of https://github.com/calliope-mini/target-calliope-mini-classic-gcc?

@Amerlander
Copy link
Collaborator Author

@riknoll we have 16 and 32mb devices, therefore we updated the yotta target to be able to change the linker file depending on the enviroment variables in makecode. Version v1.2.3 should be fine and is working on my local test.

@Amerlander
Copy link
Collaborator Author

@riknoll If you have more questions we could also set up a video call now or the next days if you like.

@riknoll
Copy link
Member

riknoll commented Mar 22, 2022

@Amerlander thanks! I think I've got a handle on things now, I'm planning on merging some changes to our backend later this afternoon to accommodate this. I'll let you know how it goes!

Out of curiosity, is there a reason that version 1.2.3 isn't merged into master?

@Amerlander
Copy link
Collaborator Author

@riknoll no, there isnt. I just merged it into master.

@riknoll
Copy link
Member

riknoll commented Mar 22, 2022

@Amerlander cloud compile is updated! The C++ portion of the build is now succeeding on master.

The build is still failing on documentation checks, though. Our CI runs through all of the TypeScript snippets in docs to make sure they compile as a check for breaking changes.

@riknoll
Copy link
Member

riknoll commented Mar 23, 2022

You can disable docs checking by editing the pxtarget.json and adding "ignoreDocsErrors": true inside the appTarget object. However, these checks are usually pretty useful because they catch errors in tutorials before they are published.

If the snippets in the docs are not meant to compile, you can also ignore them on a case-by-case basis by changing the code snippet in the markdown file to use the "typescript-ignore" language. For example:

```typescript-ignore
// code here
```

@Amerlander
Copy link
Collaborator Author

So it might be possible to disable these checks for having a public available beta version and enable them back again later?

The changes you just merged are a big improvement, but we still have a list of things that have to be changed for a final release. So our plan was to use this beta to get feedback from our community and improve it step by step from here until everything works as expected.
For that process, we plan to create issues and open separate PR to target each of them. That way we could also ask for help of the Microsoft team on issues where we are not sure how to solve them best. But everything starts with having a beta to have a common ground.

@riknoll
Copy link
Member

riknoll commented Mar 23, 2022

I think that should be fine! @abchatra FYI

@Amerlander Amerlander mentioned this pull request Mar 23, 2022
@Amerlander
Copy link
Collaborator Author

Amerlander commented Mar 23, 2022

@riknoll I created a PR for this change: #154

@riknoll I created a PR for this change: #155

@riknoll
Copy link
Member

riknoll commented Mar 23, 2022

@Amerlander the build is still a little broken, doesn't seem like it can update beta for some reason. I'm going to investigate, but in the meantime here is a build with the changes pushed:

https://makecode.calliope.cc/app/f3b8cb87d8c932e5673206a3808d227e36964416-1e24dc8c20

@Amerlander
Copy link
Collaborator Author

@riknoll thank you, I think that might be enough for now. We will start creating issues on that basis and improve it step by step, that might also solve the broken build process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants