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

fix: Change juju 3 support strategy #50

Closed

Conversation

jneo8
Copy link
Contributor

@jneo8 jneo8 commented Apr 9, 2024

  • Now all juju 3 will use 3/stable snap release
  • Update functional test to run all the supported versions by zaza
  • Drop bionic support

@jneo8 jneo8 requested a review from a team as a code owner April 9, 2024 08:40
@jneo8 jneo8 self-assigned this Apr 9, 2024
- Now all juju 3 will use 3/stable snap release
- Update functional test to run all the supported versions by
zaza
- Drop bionic support
@jneo8 jneo8 force-pushed the fix/change-juju-3.x-support-strategy branch from bbbc6bd to cf6034b Compare April 9, 2024 09:10
tox.ini Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
@jneo8 jneo8 force-pushed the fix/change-juju-3.x-support-strategy branch 2 times, most recently from 5d017f1 to 576e468 Compare April 10, 2024 07:52
@jneo8 jneo8 force-pushed the fix/change-juju-3.x-support-strategy branch from 576e468 to 0ce608d Compare April 10, 2024 08:16
Makefile Outdated Show resolved Hide resolved
.github/workflows/check.yaml Outdated Show resolved Hide resolved
tox.ini Show resolved Hide resolved
- Now it's pass from the workflow command, not from the Makefile
- Default set to 3.3/stable
@jneo8 jneo8 force-pushed the fix/change-juju-3.x-support-strategy branch from 186b194 to f0df26b Compare April 11, 2024 09:10
@jneo8 jneo8 requested a review from rgildein April 18, 2024 02:08
Makefile Show resolved Hide resolved
@dashmage
Copy link
Contributor

dashmage commented Apr 18, 2024

Since this testing strategy is a bit more different than what we're used to with other charms, it would be nice to include some documentation around this -- how we're testing the various juju versions, libjuju, zaza details, about env vars like TEST_JUJU_CHANNEL, TEST_JUJU etc.

@jneo8
Copy link
Contributor Author

jneo8 commented Apr 18, 2024

Since this testing strategy is a bit more different than what we're used to with other charms, it would be nice to include some documentation around this -- how we're testing the various juju versions, libjuju, zaza details, about env vars like TEST_JUJU_CHANNEL, TEST_JUJU etc.

I feel it's should be document in zaza, not here. Right?

@dashmage
Copy link
Contributor

Since this testing strategy is a bit more different than what we're used to with other charms, it would be nice to include some documentation around this -- how we're testing the various juju versions, libjuju, zaza details, about env vars like TEST_JUJU_CHANNEL, TEST_JUJU etc.

I feel it's should be document in zaza, not here. Right?

I was thinking more along the lines of how we are using zaza + other details specifically for these tests. But if it seems trivial, then we can ignore my comment. The changes LGTM :)

Copy link
Contributor

@dashmage dashmage left a comment

Choose a reason for hiding this comment

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

Left one small nit, otherwise LGTM

@CHARM_LOCATION=${PROJECTPATH} tox -e func33-focal -- ${FUNC_ARGS}
functional3: build
@echo "Executing functional tests using built charm at ${PROJECTPATH} with juju 3.x requirements"
@TEST_JUJU_CHANNEL=${TEST_JUJU_CHANNEL} TEST_JUJU3=1 CHARM_LOCATION=${PROJECTPATH} tox -e func3 -- ${FUNC_ARGS}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we add a comment explaining the usage of TEST_JUJU3 similar to Robert's juju 3.4 support PR?

@rgildein rgildein dismissed their stale review April 18, 2024 07:24

Do not want to block these, since we will change it again soon with 3.4 support.

@jneo8 jneo8 mentioned this pull request Apr 22, 2024
@jneo8
Copy link
Contributor Author

jneo8 commented Apr 25, 2024

Cover by #52

@jneo8 jneo8 closed this Apr 25, 2024
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.

5 participants