-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix Particle adapter #877
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
Fix Particle adapter #877
Conversation
|
@TABASCOatw the build should be passing now, can you test this out? I've changed the config API slightly to simplify the types which we can just inherit from the functions we need to call. |
c7ad500 to
f8f6f9f
Compare
Hey @jordaaash, thank you for making these changes and putting these together, big help! Just gave this a try in my testing environment + local build, seems to be working well! Tested all major functions of the adapter with the modifications made, all good. |
f8f6f9f to
e51f2cb
Compare
|
Awesome, thanks! |
|
Released via #878 |
|
@jordaaash just updated the Got this error: Guessing not all libs were updated? |
|
@vitalikda Hmm, it should be? This line specifies to use the latest workspace version on release, which is If I run |
|
@jordaaash sorry, I wrongly assumed this PR removed Also, there is a related issue opened: #880 |
|
@TABASCOatw can you make this a dependency of I don't think it makes sense for us to make |
Sure, no problem. Will check and see if we can throw it as a dependency asap; |
|
Okay, thanks! Let me know if that's released and we can update the version we depend on here. |
|
@TABASCOatw any update? We need to get this fixed or revert the changes to the Particle adapter so it doesn't break consuming projects. |
Gotcha, will share an update here before EOD. May need to go down the route of reverting and waiting for a fix, but will check again to see if we can switch those dependencies today. |
May need to revert for now (unless you'd like to add the dependency manually, but needs to be fixed on our end tbh), will put together a fix with my team asap and regroup on this. Thanks for your help here, apologies for the issues! |
|
@TABASCOatw when do you think you can get the fix in? Since this was released broken, I'll have to push a release either way, so would prefer not to have to release twice! |
Not 100% sure, pretty tight developer bandwidth at the moment on our end, so it's a bit difficult to fit this in even though it's a pretty small change, but I think we'll probably have it pushed by EOW next week, probably the next 5 or so days. |
|
@TABASCOatw Okay, it's a 1 line change to add this dependency where it should be, and it's breaking the build here. Please publish it soon! I regret merging this change and will have to roll it back otherwise. |
I've emphasized the importance of this to my team and do my best to have it merged within the next 24 hours, otherwise a rollback may be the best option if it isn't merged before then. Apologies for the issues here once again |
Update: We've pushed the new version of |
No description provided.