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

Use the DeviceOS versions endpoints when compiling #765

Merged

Conversation

suda
Copy link
Contributor

@suda suda commented Aug 21, 2024

@suda suda requested review from keeramis and hugomontero August 21, 2024 16:54
@suda suda marked this pull request as ready for review August 21, 2024 16:54
@suda
Copy link
Contributor Author

suda commented Aug 21, 2024

@keeramis do you have any insight into why the e2e tests are failing? This seemed to start after the last merge into main 🤔

@@ -154,6 +154,17 @@ module.exports = class ParticleApi {
);
}

listDeviceOsVersions(platformId, internalVersion, perPage=100){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we have different params I think might worth to transform it like {platformId, internalVersion, perPage=100} = {} unless we can't use the params separately like just send internalVersion without platformId

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. In this instance only platformId parameter is being used. I kept the positional arguments instead of making them named (with {}) to keep the function signature match the others.

Copy link
Contributor

@hugomontero hugomontero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@keeramis
Copy link
Contributor

@suda It was failing to login. Let's see if rerunning helps. I see that you are doing that already

@hugomontero
Copy link
Contributor

@keeramis do you have any insight into why the e2e tests are failing? This seemed to start after the last merge into main 🤔

In this specific case just needed to re-run tests to make it all pass 😄

@suda suda requested a review from monkbroc September 10, 2024 14:58
@suda suda force-pushed the feature/sc-129513/use-the-deviceos-versions-endpoints-when branch from dbd20b8 to f006e9a Compare September 11, 2024 15:04
@suda suda merged commit 7826f4e into master Sep 11, 2024
5 checks passed
@suda suda deleted the feature/sc-129513/use-the-deviceos-versions-endpoints-when branch September 11, 2024 15:46
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