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

Change to new release cycle #73

Merged
merged 5 commits into from Mar 8, 2022
Merged

Change to new release cycle #73

merged 5 commits into from Mar 8, 2022

Conversation

jsmassa
Copy link
Contributor

@jsmassa jsmassa commented Feb 17, 2022

No description provided.

@jsmassa jsmassa requested a review from kordano February 17, 2022 14:28
@jsmassa jsmassa self-assigned this Feb 17, 2022
Copy link
Member

@kordano kordano left a comment

Choose a reason for hiding this comment

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

For now it would be better to base these changes on the master branch since the changes for the sync+async are not yet ported in neither Datahike nor the hitchhiker-tree, and we wanted to prepare a proper release just after 0.6.0-alpha3 without the new changes.

template/pom.xml Outdated
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>io.replikativ</groupId>
<artifactId>incognito</artifactId>
Copy link
Member

Choose a reason for hiding this comment

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

The descriptions and IDs in here are still for incognito. Please adjust to konserve.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I didn't look at this file. I thought because it is a template, it is general for all libraries. I will adjust this

at: /home/circleci
- run:
name: test
command: clojure -Sthreads 1 -T:build test
Copy link
Member

Choose a reason for hiding this comment

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

I would run ./bin/run-unittests here for now since you're using still kaocha as a test dependency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

build.clj Outdated
:src-dirs ["src"])
bb/jar))

(defn test "Run the tests." [opts]
Copy link
Member

Choose a reason for hiding this comment

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

Somehow the tests just say true. Not sure whether this occurred for you as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I run the kaocha tests locally the true appears prior to the reflection warnings. It turns out, it actually originates from this line.

In fact, with this line, the tests cannot be run via the build script. I suspect that is because a :main-opt is already there so the test runner main doesn't get added (see here).
I find that quite unsatisfying because that means the reflection check has to be taken out of there and I quite liked that check in that spot. There are still libraries from us that show those warnings and it would be good to detect those things early from now on.

@whilo
Copy link
Member

whilo commented Mar 6, 2022

The hitchhiker-tree is already ported to sync+async and Datahike only needs to bump the hitchhiker-tree version. I don't see a reason to drop back to the master branch, but I might misunderstand what you are trying to do here.

@jsmassa
Copy link
Contributor Author

jsmassa commented Mar 7, 2022

At least when @kordano wrote his review, some tests of the updated hitchhiker tree were still failing, so we couldn't use this version in Datahike yet. By now, we have decided on using the new release cycle with the new changes from the dev branch and keep on using the old konserve version in the meantime though.

@jsmassa jsmassa requested a review from kordano March 7, 2022 10:44
@kordano kordano merged commit da09c0f into main Mar 8, 2022
@jsmassa jsmassa deleted the new-release-cycle branch January 18, 2023 19:00
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.

None yet

3 participants