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

refactor: replace non-empty QString constructors with QStringLiteral() #1129

Merged
merged 3 commits into from
Nov 5, 2024

Conversation

Integral-Tech
Copy link
Contributor

@Integral-Tech Integral-Tech commented Nov 3, 2024

  • Qt provides a macro QStringLiteral(), which makes constructing QString objects from string literals more efficient.

@jgehrig
Copy link
Collaborator

jgehrig commented Nov 3, 2024

@Integral-Tech

Thanks for the PR!

The changes you're making to src/auto are all auto-generated. We should revert them, and re-upload the results of the CI run: https://github.com/equalsraf/neovim-qt/actions/workflows/neovim-api.yml

It will probably yield the same code -- But we should not make manual changes to those files.

It looks like the CI is broken due to a package deprecation. I'll quickly do a version bump, so the run succeeds again.

@jgehrig
Copy link
Collaborator

jgehrig commented Nov 3, 2024

You should be all-set to pull in the changes from:
#1130

Once those changes are present in your branch, you can download the updated and auto-generated versions of the neovim-api* files from the Artifacts section of the CI run that will trigger here:
https://github.com/equalsraf/neovim-qt/actions/workflows/neovim-api.yml

After the auto-generated files are replaced, this PR looks good to me 👍

The version of upload-artifact we're currently using is deprecated:
https://github.blog/changelog/2024-02-13-deprecation-notice-v1-and-v2-of-the-artifact-actions/

We should bump to the latest version.

This also fixes a small typo in the script, where a multiline run '|'
was missing.
@Integral-Tech
Copy link
Contributor Author

@jgehrig I have pulled from master, but "ci/circleci: build" still fails

@jgehrig
Copy link
Collaborator

jgehrig commented Nov 3, 2024

Yeah, the CI needs some attention and cleaning…

You can ignore the CircleCI failures and anything else that isn’t directly related to your change.

The only one that matters here is the GitHub Actions run that generates the neovimapi code. You’ll need to download and commit those build artifacts.

@Integral-Tech
Copy link
Contributor Author

Yeah, the CI needs some attention and cleaning…

You can ignore the CircleCI failures and anything else that isn’t directly related to your change.

The only one that matters here is the GitHub Actions run that generates the neovimapi code. You’ll need to download and commit those build artifacts.

Done :)

@jgehrig jgehrig merged commit 3e4e2b7 into equalsraf:master Nov 5, 2024
23 of 24 checks passed
@Integral-Tech Integral-Tech deleted the qstring-refactor branch November 6, 2024 01:56
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