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

Regenerate proto related objects after renaming ConfigURL to ConfigRepo #1222

Merged
merged 4 commits into from
Dec 14, 2021

Conversation

josecordaz
Copy link
Contributor

@josecordaz josecordaz commented Dec 13, 2021

Closes:

What changed?

  • It wasn't mapping properly the value of the field ConfigRepo when coming from the UI
  • Regenerated proto related objects

Why?

  • Adding an app from the UI using a config repo wasn't working properly. The PR was created in the app repo instead of the config repo.
  • Some proto related objects didn't include the recent change in the field ConfigRepo

How did you test it?

  • Manually using UI to make sure it works
  • Manually using CI to make sure everything stills works

Release notes

Documentation Changes

@josecordaz josecordaz added the type/enhancement New feature or request label Dec 13, 2021
@josecordaz josecordaz changed the title Regenerate proto objects after renaming ConfigURL to ConfigRepo Regenerate proto related objects after renaming ConfigURL to ConfigRepo Dec 14, 2021
@josecordaz josecordaz marked this pull request as ready for review December 14, 2021 01:08
@josecordaz josecordaz added bug Something isn't working and removed type/enhancement New feature or request labels Dec 14, 2021
Copy link
Contributor

@JamWils JamWils left a comment

Choose a reason for hiding this comment

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

I want this tested and verified from both the CLI and UI.

@josecordaz
Copy link
Contributor Author

I want this tested and verified from both the CLI and UI.

I tested it that way

@palemtnrider
Copy link
Contributor

I see you changed the app CRD, can you verify that the resulting app CR has config_repo populated correctly.

@palemtnrider
Copy link
Contributor

Can you also make sure commits and application status work correctly with the new CR in the cluster?

@josecordaz
Copy link
Contributor Author

Can you also make sure commits and application status work correctly with the new CR in the cluster?

This is working properly, just tested it.

@josecordaz
Copy link
Contributor Author

swagger json files changed because of an update on the libraries related to the swagger generator on my local environment:

go install github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-grpc-gateway \
    github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2 \
    google.golang.org/protobuf/cmd/protoc-gen-go \
    google.golang.org/grpc/cmd/protoc-gen-go-grpc

Copy link
Contributor

@palemtnrider palemtnrider left a comment

Choose a reason for hiding this comment

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

LGTM

@josecordaz josecordaz merged commit cac2a0f into main Dec 14, 2021
@josecordaz josecordaz deleted the fix-add-with-config-and-pr branch December 14, 2021 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants