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

feat: add safari support #91

Merged
merged 6 commits into from
Feb 3, 2023
Merged

feat: add safari support #91

merged 6 commits into from
Feb 3, 2023

Conversation

acehinnnqru
Copy link
Contributor

@acehinnnqru acehinnnqru commented Feb 2, 2023

This is a Draft PR because it requires another dependency to merge a Fix PR.

This PR is going to support safari linked to issue #74.

The kooky project got a bug in Ventura and we could not read the correct cookie file. I have created a Fix PR but not been merged yet.

But I found that this project is using @j178's fork version of kooky. I am wondering if you will sync the code from source kooky. If you don't sync the code from the source repo, I may need to create the same fix for your fork.

Besides, I only test the leetgo whoami command locally. And the README will be modified after the dependency is updated.

@j178
Copy link
Owner

j178 commented Feb 2, 2023

Hi, thank you for opening this!

https://github.com/zellyn/kooky is not actively maintained anymore I think, I created a PR on my fork with your commits here j178/kooky#1 (you are still the author 😄 ), I will test and merge it tomorrow. Then we can proceed on this PR.

@acehinnnqru
Copy link
Contributor Author

Okay.

Once you merged the PR, I will modify the README. There will require some actions in MacOS Settings. So I think this PR may take another day.

@j178
Copy link
Owner

j178 commented Feb 3, 2023

Hi, j178/kooky#1 has been merged, and we can update the replace in go.mod to v0.0.0-20230203021902-e439c6617a99 to include the fix.

@acehinnnqru
Copy link
Contributor Author

Yeah, I will update it later. Thank you.

I will update this PR later.

@acehinnnqru acehinnnqru marked this pull request as ready for review February 3, 2023 07:09
@acehinnnqru
Copy link
Contributor Author

acehinnnqru commented Feb 3, 2023

@j178 I have updated this PR.

Need to mention: I only tested this feature at Ventura (Mac OS 13) on M1 Max.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@acehinnnqru
Copy link
Contributor Author

README_zh.md done

@j178 j178 merged commit 278a0cc into j178:master Feb 3, 2023
@j178
Copy link
Owner

j178 commented Feb 3, 2023

Thank you!

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