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

Update to new ycmd #4190

Merged
merged 1 commit into from Oct 3, 2023
Merged

Update to new ycmd #4190

merged 1 commit into from Oct 3, 2023

Conversation

bstaletic
Copy link
Collaborator

@bstaletic bstaletic commented Oct 2, 2023

Mostly completer upgrades, and we are now python 3.12 ready.

PR Prelude

Thank you for working on YCM! :)

Please complete these steps and check these boxes (by putting an x inside
the brackets) before filing your PR:

  • I have read and understood YCM's CONTRIBUTING document.
  • I have read and understood YCM's CODE_OF_CONDUCT document.
  • I have included tests for the changes in my PR. If not, I have included a
    rationale for why I haven't.
  • I understand my PR may be closed if it becomes obvious I didn't
    actually perform all of these steps.

Why this change is necessary and useful

Now we are python 3.12 ready. For a more detailed look into the diff:

e755af6fd Merge pull request #1712 from bstaletic/py312-bugs
ed6a49abf Merge pull request #1710 from bstaletic/llvm-17
3a88cf39d Merge pull request #1706 from bstaletic/go_cleanup
cff0e09f8 Merge pull request #1707 from bstaletic/empty_truthy_thingy
18b4d4946 Merge pull request #1709 from bstaletic/cicici
611920821 Merge pull request #1708 from bstaletic/more-ci-fixes
95fec7784 Merge pull request #1705 from bstaletic/no-bidi-chars
7d5ef4c7e Merge pull request #1702 from bstaletic/new_actions
a468dd796 Merge pull request #1703 from bstaletic/new_completer
be7441cc0 Merge pull request #1701 from bstaletic/new_java
5ab37ab2c Merge pull request #1699 from mispencer/HandleInvalidLocation
33922510b Merge pull request #1698 from AlexHYF/master
fe04f3f57 Merge pull request #1695 from puremourning/java-specify-jdtls-location
621efe89a Merge pull request #1694 from puremourning/update-rust-analyzer
e1d867a3b Merge pull request #1693 from puremourning/update-jdtls
157e8181a Merge pull request #1691 from andrzh/andrew/fix-readme-typo

This change is Reviewable

Copy link
Member

@puremourning puremourning left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 LGTMs obtained (waiting on @bstaletic)

a discussion (no related file):
Need to update README.md, it contains the clang version in a few places:

Please note that if using custom clangd or libclang it must match the
version that YCM requires. Currently YCM requires clang 16.0.1.


Copy link
Member

@puremourning puremourning left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: 0 of 2 LGTMs obtained (waiting on @bstaletic)

@codecov
Copy link

codecov bot commented Oct 2, 2023

Codecov Report

Merging #4190 (dd42e51) into master (3367b9b) will decrease coverage by 0.10%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4190      +/-   ##
==========================================
- Coverage   88.92%   88.83%   -0.10%     
==========================================
  Files          34       34              
  Lines        4398     4398              
==========================================
- Hits         3911     3907       -4     
- Misses        487      491       +4     

Copy link
Member

@puremourning puremourning left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: 1 of 2 LGTMs obtained (waiting on @bstaletic)

Copy link
Member

@puremourning puremourning left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 of 2 LGTMs obtained (waiting on @bstaletic)

Copy link
Collaborator Author

@bstaletic bstaletic left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 of 2 LGTMs obtained (waiting on @bstaletic)

a discussion (no related file):

Previously, puremourning (Ben Jackson) wrote…

Need to update README.md, it contains the clang version in a few places:

Please note that if using custom clangd or libclang it must match the
version that YCM requires. Currently YCM requires clang 16.0.1.

Done.


Copy link
Member

@puremourning puremourning left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 of 2 LGTMs obtained (waiting on @bstaletic)

Mostly completer upgrades, and we are now python 3.12 ready.
Copy link
Collaborator Author

@bstaletic bstaletic left a comment

Choose a reason for hiding this comment

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

Just to summarize:

  1. Ycmd updated, so now we are python 3.12 compatible.
  2. New tsserver requires node 14+, but we are saying 18+ because that is the only LTS version.
  3. Clangd fixed grammar in diagnostics.
  4. JDT reordered some fixits so no we need 3rd one not the 4th.
  5. CI is finally green (except for coverage being confused).

Reviewable status: 0 of 2 LGTMs obtained (and 1 stale) (waiting on @puremourning)

@bstaletic bstaletic added the Ship It! Manual override to merge a PR by maintainer label Oct 3, 2023
@mergify
Copy link
Contributor

mergify bot commented Oct 3, 2023

Thanks for sending a PR!

@mergify mergify bot merged commit bf0dbea into ycm-core:master Oct 3, 2023
10 of 13 checks passed
@bstaletic bstaletic deleted the new-ycmd branch October 3, 2023 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ship It! Manual override to merge a PR by maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants