Skip to content

Conversation

@RenFraser
Copy link

Overview

It's my hope that by releasing features more quickly to users we can decrease duplicate issues, gather even more interest in the language server and encourage more contributions from users that wouldn't typically contribute.

Testing

  • Tested using the snippet pasted below on my own branch (autobump) to check whether the expected tags were pushed to the repository.

Test Script

name: Deploy on: push: branches: - '*' jobs: deploy: runs-on: ubuntu-latest # if: github.repository == 'fwcd/kotlin-language-server' steps: - uses: actions/checkout@v3 # - name: Setup JDK # uses: actions/setup-java@v3 # with: # distribution: 'temurin' # java-version: '11' - name: Get latest tag uses: actions-ecosystem/action-get-latest-tag@v1 id: get-latest-tag - name: Bump patch tag uses: actions-ecosystem/action-bump-semver@v1 id: bump-semver with: current_version: ${{ steps.get-latest-tag.outputs.tag }} level: patch - name: Push new tag uses: actions-ecosystem/action-push-tag@v1 with: tag: ${{ steps.bump-semver.outputs.new_version }} # - uses: gradle/gradle-build-action@v2 # - name: Build distribution # run: ./gradlew :server:distZip :grammars:distZip # - name: Create release # uses: actions/create-release@v1 # id: create_release # env: # GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # with: # tag_name: ${{ github.ref }} # release_name: Version ${{ github.ref }} # draft: false # prerelease: false # - name: Upload server asset # uses: actions/upload-release-asset@v1 # env: # GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # with: # upload_url: ${{ steps.create_release.outputs.upload_url }} # asset_path: ./server/build/distributions/server.zip # asset_name: server.zip # asset_content_type: application/zip # - name: Upload grammar asset # uses: actions/upload-release-asset@v1 # env: # GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # with: # upload_url: ${{ steps.create_release.outputs.upload_url }} # asset_path: ./grammars/build/distributions/grammars.zip # asset_name: grammars.zip # asset_content_type: application/zip # - name: Deploy Docker image to GitHub Packages # uses: docker/build-push-action@v1 # with: # username: ${{ github.actor }} # password: ${{ secrets.GITHUB_TOKEN }} # registry: docker.pkg.github.com # repository: fwcd/kotlin-language-server/server # tag_with_ref: true # - name: Deploy Maven artifacts to GitHub Packages # run: ./gradlew :shared:publish :server:publish # env: # GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
@themkat
Copy link
Collaborator

themkat commented Mar 1, 2023

We also have the version on our Gradle scripts (server/build.gradle.kts after #424, and gradle.properties before that). I'm not a fan of automatic commits of file changes, so hopefully we can avoid that. Should we do anything to the build-script versions? Maybe just setting them to something to indicate a snapshot version or something similar?

Also, I see that the actions/create-release@v1 action runs still have ${{ github.ref }} as it's inputs for version. Won't that mean that in this case it will be main instead of a version? Because the PR changes the trigger from a tag to main-branch at the top of the workflow yaml. That is at least how I have always understood GITHUB_REF. Should we maybe change these to ${{ steps.bump-semver.outputs.new_version }} as well?

tags:
- '*'
branches:
- 'main'
Copy link
Owner

Choose a reason for hiding this comment

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

Hm, I think keeping the tag trigger would still be useful for manual releases (major versions et al) and would prefer not to trigger directly from branch pushes.

Since (IIRC) git pushes from GitHub Actions don't trigger a workflow, we could consider using a workflow_run trigger instead and move the bump logic into a separate workflow/YAML. That would also have the advantage of separating concerns more cleanly (version update vs. release).

Copy link
Author

Choose a reason for hiding this comment

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

Ok, that sounds good! I've extracted the tags steps into another workflow that depends on the Build workflow having successfully completed. I've also changed the Deploy workflow to depend on the Tags workflow in the same way. My thinking is that the Build workflow runs on the commit to main, then once successful, the Tag workflow runs, then the Deploy.

Though I'm not sure if that solves being able to manually trigger a release. If we want to bump the major or minor version manually, do we intend to roll it into a change by pushing a new tag at the same time? Such as 1.4.0 or 2.0.0. Or would some sort of GitHub action be better for that?

@fwcd
Copy link
Owner

fwcd commented Mar 1, 2023

I'm not a fan of automatic commits of file changes

Yes, we would effectively be adding an automated commit on top of every PR, which would add quite a bit of noise to the commit history too. If there's a way to avoid hardcoding the version at all into the Gradle project files, we might want to look into that (other languages like Swift rely entirely on git tags). I'm not sure whether it's idiomatic to omit the version from these files though.

@RenFraser RenFraser force-pushed the autobump branch 2 times, most recently from e3a26db to adab337 Compare March 4, 2023 00:54
@RenFraser
Copy link
Author

RenFraser commented Mar 4, 2023

(Addressing both)

If there's a way to avoid hardcoding the version at all into the Gradle project files, we might want to look into that

and

We also have the version on our Gradle scripts

I've added the changes to pull the last git tag and apply that to the version of all projects. I've tested that by printing out the versions in each subproject to the console:

> Configure project :grammars 1.3.1 > Configure project :server 1.3.1 > Configure project :shared 1.3.1 > Task :prepareKotlinBuildScriptModel UP-TO-DATE BUILD SUCCESSFUL in 2s 

it will be main instead of a version?

I think that you're right about the github.ref @themkat. I've pushed a change to address this too. :)

I need to re-test and will update this PR with the test results once I do it on another branch from my own fork.

@RenFraser
Copy link
Author

RenFraser commented Mar 4, 2023

I've tested it on another branch, autobump-test. To test, I've set the default branch to be the test branch, so that the workflow_run will be triggered. I've seen the Tag and Deploy workflows successfully trigger once the Build workflow had completed.

Here are the successful runs. Note that whilst the Deploy workflow did fail overall, it's just to publish to @fwcd's repository. I probably should've commented those two steps out during testing.

Created release:

The changes after testing were:

  • Use Version <tag> as the release name.
  • Rename the tag workflow's job from deploy to tag.
  • Add the - uses: actions/checkout@v3 step to the tag workflow.
  • Override the server distribution name in gradle since it was outputting server-<tag>.zip instead of the expected server.zip.
  • Removed if: github.repository == 'fwcd/kotlin-language-server' condition from deploy workflow so that contributors can test their releases easier.
@RenFraser
Copy link
Author

I've switched my default branch back to main to test that the Tag and Deploy workflows don't run on non-default branches. It looks like those steps haven't run for my latest commit test only build runs: https://github.com/RenFraser/kotlin-language-server/actions

@RenFraser
Copy link
Author

So where to from here? Do we still want to make any changes? :) @themkat @fwcd

Copy link
Collaborator

@themkat themkat left a comment

Choose a reason for hiding this comment

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

Sorry for being a bit late with replying. Was away on a business trip for a while, and is still a bit jetlagged...

I'm all for having more automated releases, as having them manually like now leads to almost never releasing. Maybe we could add an automatic action to create a release-page as well? There are several of those (or we might just use gh in the pipeline for it). Might be a future PR, as it is super easy to just create a new release from tag.

Unsure on where to go from here. As @fwcd requested a change, I suggest we wait for possible comments from them.

name: Tag
on:
workflow_run:
workflows: [Build]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I may be stupid here, but should we have a specifier to make sure this only runs on the main branch? Build runs for PRs and stuff as well.

Copy link
Author

Choose a reason for hiding this comment

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

Build runs for PRs and stuff as well.

That's true. During testing, it just runs for the default branch so that we can avoid that situation

Copy link
Collaborator

Choose a reason for hiding this comment

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

But will that not release for every PR in this repo? Out builds run for all PRs as well as main.

Copy link
Author

Choose a reason for hiding this comment

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

I forgot to reply, apologies. I didn't think so but my knowledge of GitHub actions is limited. Perhaps to be safe, we adjust the settings to be something like "only run the release workflow when we merge a PR to he main branch". I'm not sure if that's possible. I'll have to look into it

Copy link
Collaborator

Choose a reason for hiding this comment

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

I should have been more clear, and not try to be educational 😛 (I usually do questions like the above instead of stating things directly. Probably a force of habit after being a teacher for a while). If we do not specify a branch, it will in theory run for all branches. Just like you have specified type and workflows above, you can also specify branches 🙂
https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#limiting-your-workflow-to-run-based-on-branches

Copy link
Collaborator

Choose a reason for hiding this comment

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

@RenFraser , have you had any time to fix this comment? 🙂 Sorry for bothering you. Just hoping we can get something like this merged soon so we can avoid any more Kotlin 1.8.0 issues being reported.

@themkat
Copy link
Collaborator

themkat commented Mar 29, 2023

@fwcd , will you have time to look this over again after the changes you requested? Think it is best that you get to take a look before we look into merging this. (still one more comment from me that needs resolving before I'm comfortable doing so anyway). We need some sort of automatic releases, or at least releases more often. I understand you are busy a lot, and having them automatic would make life easy for us 🙂 Less issues that are easily fixed by newer versions containing fixes done in the last year. This PR can probably be a good step 1, where we can make eventual changes from there.

@fwcd
Copy link
Owner

fwcd commented Apr 13, 2023

Hey, sorry for getting back on this so late. I understand that it's a bit frustrating, but I would really like to give it some more thought before we move forward with fully automated releases. The reason I am a bit hesitant with this is for a few reasons:

  • Manual releases let us split the releases into distinct sets of features that are documented in the CHANGELOG.md. That makes it easy to get an overview over the changes to the language server without having to drill into the Git history (which is often too fine-grained to get a bigger picture). If we were to automate every release, the granularity of the releases would become diluted, we'd have to generate change log entries automatically, e.g. from merged PRs. But even PRs can vary wildly in scope (contrast a typofix with the implementation of semantic highlighting or the like).
  • While I very much appreciate all the work on the language server, I still like to keep a high-level overview over what's going on, especially considering that the extension ships under my (GitHub) name. Automating the entire release process from end-to-end is just a step I am not entirely comfortable with right now.

Also I think handling the implementation with all of the requirements (e.g. handling PR pushes correctly in every case, having the ability to release manually, maintaining a clean change log etc.) is not quite trivial, unfortunately. I've experimented a bit with this, one approach I've thought of was to consolidate the different workflows into a single file again with jobs and dependencies, something along the lines of:

jobs: build: ... release: if: (github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/') && github.repository_owner == 'fwcd' needs: [build] ... # Create tag if this is not a tag push, then create a release (perform the latter in every case) ... deploy: needs: [deploy] ...

This way the deploy would get skipped whenever the release job was skipped. But, as mentioned above, I'd like to hold off on progressing on this for now.

at least releases more often

I think that's a fair point and I will try to release more often. I'll put together a release this evening.

@themkat
Copy link
Collaborator

themkat commented Apr 13, 2023

I think that's a fair point and I will try to release more often. I'll put together a release this evening.

That is super nice! I noticed that I suddenly had access rights to push tags, which failed before. Tried to make a release (by pushing a tag), but it failed. Seems like versions are included in the distZip zip results... (e.g, server-1.3.2.zip). Reverted my tag (deleted it).

Again, sorry for nagging you about releases!! We are just super eager about this project! 🙂

EDIT: Found a workaround to get the filename to be server.zip again. Setting the projectVersion in server/build.gradle.kts to the empty string. Not ideal, but might be a quick workaround to get a release out the door. Might need to do something similar for grammars.

@fwcd
Copy link
Owner

fwcd commented Apr 13, 2023

No need to do that, I wanted to update the release workflow anyway to use the gh CLI instead of the deprecated action, so I'll just update the CI instead (including the version in the archive name seems useful to me).

@RenFraser
Copy link
Author

Thanks for getting back to me. I'll cancel this PR. If you're uncomfortable with continuous releases at this stage, perhaps making use of GitHub's pre-release mechanism would be useful instead?

Manual releases let us split the releases into distinct sets of features that are documented in the CHANGELOG.md

I wonder if you would be interested in Conventional Commits? One of the major benefits is that it automatically generates a changelog for you. The Neovim repository has used it to good success for a long time now.

@RenFraser RenFraser closed this Apr 15, 2023
@RenFraser RenFraser deleted the autobump branch August 1, 2023 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants