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

(Fixed) IR Additions #543

Merged
merged 60 commits into from
Aug 30, 2023
Merged

(Fixed) IR Additions #543

merged 60 commits into from
Aug 30, 2023

Conversation

emptythevoid
Copy link

The only thing I would like to ignore with the linter, if possible, is for the GPX_D2816.ir file. The remote actually has printed "Prev" and "Next." I can change them, if you'd prefer, but I thought I'd explain first.

Otherwise, I think everything else is fixed.

Remote has a CD/CDR switch, so there are two remote files.
Remote has two modes, so two separate files
Remote files are for Deck A and Deck B
Remotes have a mode switch to toggle between four different cameras.  I have captured each one as it's own .ir file
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🐛 Linter Result

DVD_Players/GPX/GPX_D2816.ir [2 issues]

# Line 278:
- name: Prev
        ^^^^
@@ ambiguous name 'Prev' @@
# Line 284:
- name: Next
        ^^^^
@@ ambiguous name 'Next' @@

@darmiel
Copy link
Member

darmiel commented Aug 30, 2023

Thank you for your additions and for fixing the previous issues 🙂

The linter only complains about the names because they occur twice (2x Prev, 2x Next), which can lead to confusion.
Is it possible that the duplicates are Fast Forward/Backward ? On a picture of the remote I saw the following buttons:

image

But if you say it's correct as it is now, that's okay too and I can go ahead and merge it as it currently is.

P.S. you don't need to close and create a new pull request when you update something. Just push the changes to the branch you created the PR from (in your case iradditions) and the pull request will automatically update too 🙂

I fixed the fix I didn't fix right
@emptythevoid
Copy link
Author

I can't do a good job explaining, except I think I tried to fix it from the linter, decided against it, and un-fixed the wrong thing. Does it look OK now?

@emptythevoid
Copy link
Author

(And thank you for helping me. I'm not used to using a proper github workflow with others)

@darmiel
Copy link
Member

darmiel commented Aug 30, 2023

Yes, looks good now 👍

@darmiel darmiel merged commit 931bce7 into logickworkshop:main Aug 30, 2023
1 check passed
@emptythevoid
Copy link
Author

Thank you very much!

@emptythevoid emptythevoid deleted the iradditions branch August 30, 2023 20:40
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.

2 participants