Skip to content

Use GH API to collect versions from remote test harnesses #1916

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

Merged
merged 7 commits into from
May 29, 2025

Conversation

OptimumCode
Copy link
Collaborator

@OptimumCode OptimumCode commented Mar 26, 2025

The PR adds a collection of version information from remote test harnesses. It collects all available version tags (like harness-release-*), plus it uses the latest tag to get the latest available test harness version.

Related to #1849


📚 Documentation preview 📚: https://bowtie-json-schema--1916.org.readthedocs.build/en/1916/

@Julian
Copy link
Member

Julian commented May 15, 2025

@OptimumCode what did we leave off on here? I'm sure it's my fault that this PR stalled (really sorry!) if you have a moment let me know what I needed to do here (was it just it's ready for a final review / merge)?

@OptimumCode
Copy link
Collaborator Author

It is not your fault @Julian ) The actions are on my side - I need to modify the workflow in the harness repository so it does not generate tags like vv1.2.3 and modify this workflow to do the same (in case of latest tag). I have recently switched jobs and cannot find a time to finish that yet, unfortunately... Hopefully, I will find time in a few weeks...

Re PR itself, I think it will be ready for review once I (or somebody else) complete the above actions (if we are okay to keep current logic that collects tags until #1928 is implemented).
After that, it looks like this is the final step after which we can proceed with migration of other harnesses (create template repo with workflows and configs and start moving harnesses to separate repos)

@OptimumCode
Copy link
Collaborator Author

After the following change (bowtie-json-schema/kotlin-kmp-json-schema-validator#20), the tag for the test harness will start with a harness-release- suffix. That should reduce the chance of collision with user-defined tags and with implementation versions that start with v

@OptimumCode
Copy link
Collaborator Author

The PR is ready for review now. Could you please take a look, @Julian, once you have time?

Copy link
Member

@Julian Julian left a comment

Choose a reason for hiding this comment

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

I haven't tried to run this, just glanced at it, but if you've run it and it works this is good to merge with me. Just left 2 minor comments.

@OptimumCode
Copy link
Collaborator Author

Thank you, @Julian! I have ran it partially, will try to run the whole workflow and if all is good, I will merge the changes. I will do that closer to the end of the week (and make changes according to your comments)

@OptimumCode
Copy link
Collaborator Author

Hi @Julian, I have tried to execute the whole workflow locally, and it looks like I am missing something because I am getting an error on the following command:

mkdir "$impl/v$version" # when dialect.json is created for each version

mkdir: kotlin-kmp-json-schema-validator: Not a directory

A few lines above, we copy the matrix version file to the current dir with the name equal to the implementation name

cp $MATRIX_VERSIONS_FILE $impl

Now, we have a file with the implementation name, and because of that, we cannot create a directory with the same name. But I don't see any errors in the workflow execution on GitHub, meaning it works okay there. Have you happened to know how it works on GitHub runner? Maybe we should correct the script a bit so we can execute the same code locally if needed?

@OptimumCode
Copy link
Collaborator Author

Hi, @Julian! Just wanted to check if you had a chance to take a look at my last comment. I think this uncertainty is the only thing that blocks from merging the PR (because I honestly have no idea how it works right now on GitHub, and don't want to break anything since I was not able to run the workflow completely)

@Julian
Copy link
Member

Julian commented May 28, 2025

I saw it but I didn't have any idea either without trying it out myself :) at a first glance you looked right but as you say it works here so clearly we're misreading the shell code and it's doing something slightly different (which is why I always want to get rid of it and exchange it for working and tested Python code)

I don't know when I'll get a chance to run it myself, I have a huge number of things on my plate to get to -- I'd say if it's not obvious to the two of us but works here it's possibly still fine to merge, as we must be missing something simple...

@OptimumCode
Copy link
Collaborator Author

Agree, let's wait until weekend - I will merge the changes and try to execute the updated workflow. If something doesn't work - I will prepare a PR with fix

@Julian
Copy link
Member

Julian commented May 28, 2025

Sounds great, thanks again for all your hard work!

@OptimumCode OptimumCode force-pushed the collect-remote-versions-for-trends branch from 0c1fc6b to 94ce62d Compare May 29, 2025 09:23
@OptimumCode OptimumCode merged commit 76d7161 into main May 29, 2025
46 of 48 checks passed
@OptimumCode OptimumCode deleted the collect-remote-versions-for-trends branch May 29, 2025 09:44
@OptimumCode
Copy link
Collaborator Author

I forgot I have a day off today) I have found a small issue and fixed it in #2020. The rest looks pretty good - I can see the trends chart for external test harness (https://bowtie.report/#/implementations/kotlin-kmp-json-schema-validator).

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