Skip to content

Extension: add Smart Hydroponics Kit #6313

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

Merged
merged 2 commits into from
Jun 19, 2025

Conversation

martinwork
Copy link
Contributor

@abchatra
Copy link
Collaborator

@ssande-fwd do you need to hardcode the dependency here? If you do that any bug fixes in those extensions will not be carried over.

    "fwd-float": "github:Forward-Education/pxt-fwd-modules/fwd-float#v1.0.5",
    "fwd-ph": "github:Forward-Education/pxt-fwd-modules/fwd-ph#v1.0.5",
    "fwd-lights": "github:Forward-Education/pxt-fwd-modules/fwd-lights#v1.0.5",
    "fwd-pump": "github:Forward-Education/pxt-fwd-modules/fwd-pump#v1.0.5"

@ssande-fwd
Copy link

ssande-fwd commented Jun 18, 2025

I tried to do it without specifying version and it was not 100% reliable. Once it stopped fetching the latest version of a dependency I could never fix it. It happened a few times in a few different repos. I opened a ticket about it and was told to always specify the version because when you don't, behavior is undefined (I know its supposed to pull latest tag or latest commit if no tags).

@abchatra
Copy link
Collaborator

@ssande-fwd can you point to the ticket?

@ssande-fwd
Copy link

#6271 (comment)

@abchatra
Copy link
Collaborator

@ssande-fwd We do cache the extensions for 24 hours in the CDN and local cache. So hardcoding is good for testing if you want to test immediately after bumping one after another. However, if you are releasing after testing, I would suggest not hardcoding unless you are worried someone else would change those dependencies without testing. Does it make sense? I will merge this for now.

@abchatra abchatra enabled auto-merge (squash) June 19, 2025 06:09
@abchatra abchatra disabled auto-merge June 19, 2025 06:10
@abchatra abchatra enabled auto-merge (squash) June 19, 2025 06:10
@abchatra abchatra disabled auto-merge June 19, 2025 06:12
@abchatra abchatra merged commit dfcc570 into microsoft:master Jun 19, 2025
11 checks passed
@ssande-fwd
Copy link

ssande-fwd commented Jun 19, 2025

The fetching of the wrong version persisted for days. I can make another issue if it happens again, but I don't see much incentive to not specify the version. I'll take stability and reliability over convenience, given the frustration I've experienced when it randomly doesn't work as expected and can not be fixed. Martin can correct me if I'm wrong but I think it's even part of the coding standards. I recall him asking me to specify a dependency version where I did not. Thanks for merging. Like I said I wanted to do it the way you are suggesting but I got pushback in several forms and from several directions.

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.

3 participants