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.

Sign up for GitHub

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

Jump to bottom

fix(bazel): Handle comment and string parsing #8794

Closed
wants to merge 9 commits into from
Closed

fix(bazel): Handle comment and string parsing #8794

wants to merge 9 commits into from

Conversation

zharinov
Copy link
Collaborator

Changes:

Use moo lexer to handle sophisticated parsing edge-cases (comments and strings).

Context:

Closes #8785

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please tick one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added unit tests, or
  • No new tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

zharinov
zharinov commented Feb 21, 2021
View reviewed changes
lib/manager/bazel/extract.ts
'http_file',
'go_repository',
'git_repository',
].reduce(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This iteration caused so huge diff on rewritten snapshot (note other snapshot tests are same).

@zharinov
Copy link
Collaborator Author

Can someone please point to good real use-case to test against?

viceice
viceice previously approved these changes Feb 22, 2021
View reviewed changes
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

Looks good if it works ☺️

Sergei Zharinov added 2 commits February 22, 2021 16:41
@zharinov
Copy link
Collaborator Author

Example repo: https://github.com/renovate-testing/cockroach-gen/pulls

Works similarly this branch and master. I guess, this one should work good enough to fix the original issue and not break existing behavior.

@zharinov zharinov requested a review from viceice February 22, 2021 14:04
viceice
viceice previously approved these changes Feb 22, 2021
View reviewed changes
@rarkins
Copy link
Collaborator

rarkins commented Feb 22, 2021

@zharinov I think this is the second time that this PR has failed tests with out of memory limits. Is there a chance it's caused by the code in this branch?

@zharinov zharinov marked this pull request as draft February 22, 2021 16:36
@zharinov
Copy link
Collaborator Author

I've noticed submodules test timeout for another branch. But for memory allocations, I'm not sure.

@viceice
Copy link
Member

viceice commented Feb 22, 2021

I've seen this on my pr too. Maybe we should enable jest mem stats again.

@zharinov zharinov force-pushed the fix/bazel-parser-comments-and-strings branch from d36b490 to 2aeca0c Compare February 22, 2021 18:51
@zharinov zharinov force-pushed the fix/bazel-parser-comments-and-strings branch from 2aeca0c to 0cd1ce7 Compare February 22, 2021 18:52
@zharinov
Copy link
Collaborator Author

image

Sergei Zharinov added 2 commits February 23, 2021 20:51
@zharinov
Copy link
Collaborator Author

This PR is cursed, I guess

@zharinov zharinov closed this Feb 23, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@viceice viceice viceice left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Parsing Bug in Bazel Extract
3 participants
@zharinov @rarkins @viceice