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: Controller resource set before version #744

Merged
merged 1 commit into from
May 20, 2022
Merged

fix: Controller resource set before version #744

merged 1 commit into from
May 20, 2022

Conversation

LuukvH
Copy link
Contributor

@LuukvH LuukvH commented Feb 22, 2022

The resource description is set before the controller versions are defined.
When the namespaced_resources option is enabled this will lead to incorrect resource naming.

@mathieujobin
Copy link
Collaborator

can you explain ? is it possible to write a test that reproduce the error this is fixing ?

@mathieujobin
Copy link
Collaborator

also please update your branch to be up to date, so the tests runs

The resource description is set before the controller
versions are defined.
When the namespaced_resources option is enabled
this will lead to wrong resource naming.
@mathieujobin
Copy link
Collaborator

Thank you for updating your branch... I would not mind merging, but I need to understand what this fixes?
Is there a test that could be written that reproduce the problem that current master has and this fixes?

Copy link
Collaborator

@mathieujobin mathieujobin left a comment

Choose a reason for hiding this comment

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

I need to understand what problem this changes fix... ideally a test should be added

@LuukvH
Copy link
Contributor Author

LuukvH commented May 20, 2022

@mathieujobin I am not able to write a test for it, since we have decided to move away from apipie due to the state it is in. What it fixes is that the resource name generated when no version is present is different than the one when the version is present. To prevent this we should make sure that the controller versions are set before defining the resource description.

@mathieujobin mathieujobin merged commit e86c30e into Apipie:master May 20, 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.

2 participants