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

Spoon: Watch for meeting #228

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open

Conversation

asp55
Copy link

@asp55 asp55 commented Apr 7, 2021

Submitting WatchForMeeting.spoon, which monitors for active zoom.us calls, and can share that information to a web page (that can in turn be run on a tablet outside the room to let others know you're busy.)

@asp55
Copy link
Author

asp55 commented Apr 7, 2021

Is PR / build failing because of this warning:

WARN: SIGNATURE/PARAMETER COUNT MISMATCH: 'WatchForMeeting:fake(mic_open, video_on, sharing)' says 3 parameters ('mic_open, video_on, sharing'), but Parameters section has 0 entries:

Not sure I understand why it's getting that warning...

My documentation has those 3 parameters spelled out:

--- WatchForMeeting:fake(mic_open, video_on, sharing)
--- Method
--- Disables monitoring and reports as being in a meeting. 
--- Useful when meeting type is not supported (currently any platform that isn't zoom.)
---
--- Parameters:
--- * mic_open: Boolean
--- * video_on: Boolean
--- * sharing: Boolean
---
--- Returns:
---  * The spoon.WatchForMeeting object

And they seem to all be there in docs.json

@asp55
Copy link
Author

asp55 commented Apr 7, 2021

Never mind, that was exactly why it was failing. Seems like that check is very picky about the formatting. Had to rewrite parameters as

--- Parameters:
---  * mic_open - A boolean indicating if the mic is open
---  * video_on - A boolean indicating if the video camera is on
---  * sharing - A boolean indicating if screen sharing is on

And now we're passing.

@cmsj
Copy link
Member

cmsj commented Apr 7, 2021

@asp55 sorry it’s so picky. I’m going to look into having the docstring checker leave specific comments on the PR to show where the problems are. Might make it easier for people to deal with.

@asp55
Copy link
Author

asp55 commented Apr 9, 2021

@cmsj No problem, I eventually got it.

Though I'm not entirely sure what I fixed to clear the first set of warnings that claimed this about all my methods...

WARN: SIGNATURE/PARAMETER COUNT MISMATCH: 'WatchForMeeting:start()' says 1 parameters (''), but Parameters section has 0 entries:

@asp55
Copy link
Author

asp55 commented May 20, 2021

I don't think this check is failing because of something in my code. Or am I missing something?

Failing check: Publish Docstring Annotations

1
Run yuzutech/annotations-action@v0.3.0
8
Creating check {owner: 'Hammerspoon', repo: 'Spoons', name: Docstrings linter}
9
Error: GitHubApiUnauthorizedError: Unable to create a check, please make sure that the provided 'repo-token' has write permissions to 'Hammerspoon/Spoons' - cause: HttpError: Resource not accessible by integration

@cmsj
Copy link
Member

cmsj commented Jan 17, 2022

@asp55 If you rebase this on top of current master in this repo, you should get proper feedback from the docstrings linter.

@asp55
Copy link
Author

asp55 commented May 31, 2022

@cmsj yep, seems to be working now.

@asp55
Copy link
Author

asp55 commented Aug 5, 2023

Added a few new features to this spoon.

@asp55
Copy link
Author

asp55 commented Apr 4, 2024

Hi, checking in if this pull request is being considered.

asp55 added 3 commits June 28, 2024 14:45
…. Zoom updated their menus to use sentance casing rather than title casing, and findMenuItem is case sensetive.
@cmsj cmsj closed this Aug 7, 2024
@cmsj cmsj reopened this Aug 7, 2024
@cmsj
Copy link
Member

cmsj commented Aug 7, 2024

@asp55 apologies for the heinous delay in look at this. Looks like there's one docstrings nit to take care of, and then this can merge.

Copy link
Contributor

@muescha muescha left a comment

Choose a reason for hiding this comment

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

Added some nitpicks

Source/WatchForMeeting.spoon/init.lua Outdated Show resolved Hide resolved
Source/WatchForMeeting.spoon/init.lua Outdated Show resolved Hide resolved
Source/WatchForMeeting.spoon/init.lua Outdated Show resolved Hide resolved
Source/WatchForMeeting.spoon/init.lua Show resolved Hide resolved
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