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

feat: Re-add support for Qt Creator 15.0.1 #130

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 17 additions & 8 deletions .github/workflows/build_cmake.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ on:
env:
PLUGIN_NAME: QodeAssist
QT_VERSION: 6.8.2
QT_CREATOR_VERSION: 16.0.0
QT_CREATOR_VERSION_INTERNAL: 16.0.0
MACOS_DEPLOYMENT_TARGET: "11.0"
CMAKE_VERSION: "3.29.6"
NINJA_VERSION: "1.12.1"
Expand Down Expand Up @@ -47,6 +45,17 @@ jobs:
platform: mac_x64,
cc: "clang", cxx: "clang++"
}
qt_creator:
- {
version: "15.0.1",
Copy link
Owner

Choose a reason for hiding this comment

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

I am not sure about support old version, maybe are leave only current for now and in future add new like beta?

Copy link
Contributor Author

@p12tic p12tic Mar 13, 2025

Choose a reason for hiding this comment

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

Why not support older version as well? In our codebase the difference is 10 lines, it's not likely to raise any issues.

It's problematic to force users to upgrade to newest versions of Qt Creator because other third-party extensions may not upgrade as quickly. Also, new Qt Creator versions may contain bugs that break workflows and thus aren't usable. I've been burned by these multiple times. Both of these are barriers to use QodeAssist.

I would say that it makes sense to support all releases, especially since this PR has already done the work. If we actually see maintenance overhead, such as bugs that are specific to single version, then it makes sense to re-evaluate supporting fewer versions.

Copy link
Owner

Choose a reason for hiding this comment

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

In that case, we probably need to take the Qt version as part of it.

Copy link
Owner

@Palm1r Palm1r Mar 13, 2025

Choose a reason for hiding this comment

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

I checked, yes, idea is good but it ieads more changes, for artifacts name, for check and download new version of plugin and plugins.json(indeed I don't know reason of this file like a collect data of plugins) and for usual PR we don't need to check all versions of QtC, only current I think. So lets do it later by another PR and just update now to new QtC version, because we don't have new features for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for usual PR we don't need to check all versions of QtC, only current I think.

I think this doesn't matter because each build uses very little resources. It finishes in only couple of minutes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed with the rest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for check and download new version of plugin

This one already works.

Copy link
Owner

@Palm1r Palm1r Mar 16, 2025

Choose a reason for hiding this comment

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

Not exactly, if we add new multiply versions, which version will be download, plus Qt version, QtCreator 15 based on 6.8.1

version_internal: "15.0.1",
latest: false
}
- {
version: "16.0.0",
version_internal: "16.0.0",
latest: true
}

steps:
- uses: actions/checkout@v4
Expand Down Expand Up @@ -176,7 +185,7 @@ jobs:
- name: Download Qt Creator
uses: qt-creator/[email protected]
with:
version: ${{ env.QT_CREATOR_VERSION }}
version: ${{ matrix.qt_creator.version }}
unzip-to: 'qtcreator'

- name: Extract Qt Creator
Expand Down Expand Up @@ -223,7 +232,7 @@ jobs:
COMMAND python
-u
"${{ steps.qt_creator.outputs.qtc_dir }}/${build_plugin_py}"
--name "$ENV{PLUGIN_NAME}-$ENV{QT_CREATOR_VERSION}-${{ matrix.config.artifact }}"
--name "$ENV{PLUGIN_NAME}-${{ matrix.qt_creator.version }} -${{ matrix.config.artifact }}"
--src .
--build build
--qt-path "${{ steps.qt.outputs.qt_dir }}"
Expand All @@ -241,12 +250,12 @@ jobs:
- name: Upload
uses: actions/upload-artifact@v4
with:
path: ./${{ env.PLUGIN_NAME }}-${{ env.QT_CREATOR_VERSION }}-${{ matrix.config.artifact }}.7z
name: ${{ env.PLUGIN_NAME}}-${{ env.QT_CREATOR_VERSION }}-${{ matrix.config.artifact }}.7z
path: ./${{ env.PLUGIN_NAME }}-${{ matrix.qt_creator.version }}-${{ matrix.config.artifact }}.7z
name: ${{ env.PLUGIN_NAME}}-${{ matrix.qt_creator.version }}-${{ matrix.config.artifact }}.7z

# The json is the same for all platforms, but we need to save one
- name: Upload plugin json
if: startsWith(matrix.config.os, 'ubuntu')
if: startsWith(matrix.config.os, 'ubuntu') && matrix.config.latest
uses: actions/upload-artifact@v4
with:
name: ${{ env.PLUGIN_NAME }}-origin-json
Expand Down Expand Up @@ -286,7 +295,7 @@ jobs:
QT_TOKEN: ${{ secrets.TOKEN }}
API_URL: ${{ secrets.API_URL }}
run: |
node .github/scripts/registerPlugin.js ${{ env.RELEASE_HTML_URL }} ${{ env.PLUGIN_NAME }} ${{ env.QT_CREATOR_VERSION }} ${{ env.QT_CREATOR_VERSION_INTERNAL }} ${{ env.QT_TOKEN }} ${{ env.API_URL }}
node .github/scripts/registerPlugin.js ${{ env.RELEASE_HTML_URL }} ${{ env.PLUGIN_NAME }} ${{ matrix.qt_creator.version }} ${{ matrix.qt_creator.version_internal }} ${{ env.QT_TOKEN }} ${{ env.API_URL }}

- name: Delete previous json artifacts
uses: geekyeggo/delete-artifact@v5
Expand Down
Loading