-
Notifications
You must be signed in to change notification settings - Fork 59
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
Open in new window. #14
Conversation
Closes #8 |
Hello jtagcat, thank you for your contribution! I will do my best to review within the upcoming weeks. In regard to taking over: The extension has >130.000 users, I feel responsible for providing a stable version of the extension to them. Additionally, to me the extension is nearly feature-complete. For these reasons I would rather you fork the project. |
As long as you keep active on PRs ;) It just seemed that the only time you were active ever on github was oct-dec last year. Regex and settings sync is not fully complete. I think I finished regex, but need to cleanup, sync should in theory work, but I have to find a way to test it on both chrome and ff so the settings actually keep themselves (by current testing, when you check the sync storage, you will get 'true' for everything). |
Couldn't get it to work in 40 minutes, reverting sync mechanism to the one currently in master. Firefox support (#15) remains broken. As for regex, current vs the gold standard compatible with standards (from SO). There's problems with escaping and stuff, I'm not very exited to fix it. The MR should be ready to merge. |
I apologize for this pull. I should've talked to you more and made smaller pulls; instead of trying to do everything within days of working on the same thing. Since I'm already somewhat familiar, I'll try again some time. When is some time, I don't know. |
Next time, comment your code more please.
Tested on Firefox and Chrome.
Closes #7
I am ok to take over and become a maintainer.