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

Switch to dschmidt/easygettext to use typescript-estree #6833

Merged
merged 1 commit into from
Apr 28, 2022

Conversation

dschmidt
Copy link
Member

@dschmidt dschmidt commented Apr 28, 2022

…from typescript-eslint project instead of outdated buntis parser

Description

buntis is outdated and fails to parse newer javascript/typescript syntax like optional chaining.
There is a PR open for porting to the properly maintained typescript-eslint/typescript-estree library, but it hasn't seen any feedback. It does not fix linenumbers anyhow.
I've sent a new PR which also fixes line numbers, I'm not optimistic it will be merged instead ... we should consider creating a fork in owncloud organization. Let's discuss tomorrow...

Related Issue

Motivation and Context

It fixes missing line numbers and parse errors in ts files/vue files using typescript.

How Has This Been Tested?

removed .pot files from .gitignore
used old version to create those files, commited them. used new version and looked at the diff: only a few line numbers diverged slightly, most probably nothing to worry about.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Open tasks:

  • I don't like to make projects depend on my personal fork, should we create a fork repo in owncloud organization?

@update-docs
Copy link

update-docs bot commented Apr 28, 2022

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

Copy link
Contributor

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

let's merge this to unblock nightly translation sync jobs. good as intermediate solution 👍

@kulmann kulmann merged commit 607a0d0 into master Apr 28, 2022
@delete-merged-branch delete-merged-branch bot deleted the use_easygettext_fork branch April 28, 2022 21:18
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@kulmann
Copy link
Contributor

kulmann commented Apr 28, 2022

Grmpf. I waited with the force merge until all the build steps were done in CI run https://drone.owncloud.com/owncloud/web/25001
now I have issues when running yarn install locally, essentially this: npm ERR! 403 403 Forbidden - GET https://npm.polydev.blue/safe-buffer/-/safe-buffer-5.2.0.tgz - unregistered users are not allowed to access package safe-buffer during Packing easygettext@https://github.com/dschmidt/easygettext.git#commit=da6f3d7ac03345ecec44a2694957533bc56c0f15 from sources

@dschmidt
Copy link
Member Author

That was super weird. The lock file contained the npm.polydev.blue registry (for whatever that is). I've changed it now to point to registry.npmjs.org and everything seems fine without SHA changes .... so this is apparently not malicious - just weird

@kulmann
Copy link
Contributor

kulmann commented Apr 28, 2022

Updated in d486455 - thanks for reacting so fast! ❤️

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