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

JBL driver clean commit #958

Closed
wants to merge 5 commits into from
Closed

JBL driver clean commit #958

wants to merge 5 commits into from

Conversation

varzac
Copy link
Contributor

@varzac varzac commented Sep 14, 2023

This is a re-upload of #842 to test the latest CI changes.

@varzac varzac requested review from dljsjr and cjswedes September 14, 2023 19:10
@github-actions
Copy link

Duplicate profile check: Passed - no duplicate profiles detected.

@github-actions
Copy link

github-actions bot commented Sep 14, 2023

Channel deleted.

@github-actions
Copy link

github-actions bot commented Sep 14, 2023

Test Results

     53 files     340 suites   0s ⏱️
1 611 tests 1 611 ✔️ 0 💤 0
2 794 runs  2 794 ✔️ 0 💤 0

Results for commit 9822df5.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Sep 14, 2023

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against 9822df5

Copy link

@chillcyj chillcyj left a comment

Choose a reason for hiding this comment

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

Hi @varzac
Could you pull the latest code from main (https://github.com/cchillerr/SmartThingsEdgeDrivers), we have just applied a patch to improve the performance of driver. Thanks.

@varzac
Copy link
Contributor Author

varzac commented Sep 19, 2023

Hi @varzac Could you pull the latest code from main (https://github.com/cchillerr/SmartThingsEdgeDrivers), we have just applied a patch to improve the performance of driver. Thanks.

You can do this in your own branch, you just need to rebase to include all the latest changes from the main repo in your repo. I pulled in your changes but your code does not currently pass the linter

Checking drivers/SmartThings/jbl/src/discovery.lua 2 warnings

    drivers/SmartThings/jbl/src/discovery.lua:63:9: (W211) unused variable unknown_discovered_devices
    drivers/SmartThings/jbl/src/discovery.lua:64:9: (W211) unused variable known_discovered_devices

@chillcyj

@chillcyj
Copy link

Hi @varzac
The rebase is done, the unused variables are removed. Please have a check, thanks.

@varzac
Copy link
Contributor Author

varzac commented Sep 21, 2023

Hi @varzac The rebase is done, the unused variables are removed. Please have a check, thanks.

Thank you! I have updated this PR to include the latest, and I have also run the CI on your PR so both channel invites should use the correct driver now.

@lelandblue
Copy link
Contributor

Closing as this went to prod as #842

@lelandblue lelandblue closed this Oct 18, 2023
@dljsjr dljsjr deleted the fresh-jbl branch December 18, 2024 15:19
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