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

[JUJU-796] Add relate method and deprecate add-relation #660

Merged
merged 1 commit into from
Apr 26, 2022

Conversation

jack-w-shaw
Copy link
Member

Description

This is to follow the pattern we are following going ahead with Juju.
change the name of add-relation to relate, but keep add-relation, which
calls relate for backwards compatibility

This method should then be removed at the next major version bump

See this PR for more details
juju/juju#13907

QA Steps

tox -e integration
tox -e unit

@jack-w-shaw jack-w-shaw force-pushed the JUJU-796_use_relate branch from acbbfc7 to 09a5e58 Compare March 30, 2022 11:21
@juanmanuel-tirado juanmanuel-tirado added the hint/do-not-merge requires additional efforts, no merge label Mar 30, 2022
@juanmanuel-tirado
Copy link
Contributor

I label this as "do-not-merge" until a we have a consistent message in the juju CLI.

@jack-w-shaw jack-w-shaw force-pushed the JUJU-796_use_relate branch from 09a5e58 to 3caaf1b Compare March 30, 2022 12:04
@cderici
Copy link
Contributor

cderici commented Mar 30, 2022

!!build!!

Copy link
Contributor

@cderici cderici left a comment

Choose a reason for hiding this comment

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

Other than my comments about the duplication in tests, this lgtm, the CI failures seem to be intermittent glitches, I think we're good there too 👍

tests/integration/test_crossmodel.py Show resolved Hide resolved
tests/integration/test_model.py Show resolved Hide resolved
@jack-w-shaw jack-w-shaw force-pushed the JUJU-796_use_relate branch from 3caaf1b to 7d89a38 Compare March 31, 2022 13:43
Copy link
Contributor

@cderici cderici left a comment

Choose a reason for hiding this comment

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

This lgtm. The CI fails are unrelated to this PR, so you're clear for landing.

This is to follow the pattern we are following going ahead with Juju.
change the name of add-relation to relate, but keep add-relation, which
calls relate for backwards compatibility

This method is then to be removed at the next major version bump

See this PR for more details
juju/juju#13907
@jack-w-shaw jack-w-shaw force-pushed the JUJU-796_use_relate branch from 7d89a38 to d810ec6 Compare April 26, 2022 14:40
@cderici cderici removed the hint/do-not-merge requires additional efforts, no merge label Apr 26, 2022
@cderici
Copy link
Contributor

cderici commented Apr 26, 2022

I just ran the CI manually for this one and all the tests pass, this is clear for landing 👍

@jameinel jameinel merged commit 768d7ff into juju:master Apr 26, 2022
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.

4 participants