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

Replace all instances of yomichan #314

Merged
merged 7 commits into from
Nov 12, 2023
Merged

Replace all instances of yomichan #314

merged 7 commits into from
Nov 12, 2023

Conversation

koiyakiya
Copy link

@koiyakiya koiyakiya commented Nov 11, 2023

This PR replaces (to my knowledge) every instance of yomichan to yomitan in the project (that is necessary). I ran the tests and they gave back no errors. I did a little testing and everything seems to work.

Some things to note:

  • One problem I did have was exporting or importing dictionary collections, but the error it gave back probably has to do with me not setting up the enviornment right (it gave a TypeError: Dexie is not a constructor).
  • Also, in the Errors tab in chrome extensions, it gives an Uncaught SyntaxError: Cannot use import statement outside a module, and links js/language/text-scanner.js:19 (anonymous function). Don't really know what this means...
  • Other than that, Anki works, dictionaries import correctly, audio works, formatting is normal, etc. (Only tested on chrome)
  • In templates.md, I changed the handlebars examples to yomitan. For example, {{~concat "yomitan_" (get "index") "_value"}}. I'm not sure if this is intended and if it is correct in the code.

If I did something wrong, please tell me, as this is my first time making a PR of this size...

(also the participant aiborii is just me, I think I messed with git a bit...)

@koiyakiya koiyakiya marked this pull request as ready for review November 11, 2023 01:57
@koiyakiya koiyakiya requested a review from a team as a code owner November 11, 2023 01:57
Copy link

github-actions bot commented Nov 11, 2023

⚠️ Visual differences introduced by this PR; please validate if they are desirable.

View Playwright Report (note: open the "playwright-report" artifact)

@jbukl
Copy link

jbukl commented Nov 11, 2023

⚠️ Visual differences introduced by this PR; please validate if they are desirable.

View Playwright Report (note: open the "playwright-report" artifact)

just noting the playwright tests didn't actually fail - it's just detecting the title of the test page changing to yomitan
and the "Cannot use import statement outside a module" should be unrelated - I opened a PR regarding that
same with dexie

djahandarie
djahandarie previously approved these changes Nov 12, 2023
@djahandarie djahandarie added this pull request to the merge queue Nov 12, 2023
Merged via the queue into yomidevs:master with commit d06029e Nov 12, 2023
6 checks passed
@djahandarie djahandarie added the kind/meta The issue or PR is meta label Nov 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/meta The issue or PR is meta
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants