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.

Sign up for GitHub

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

Jump to bottom

Feat/update protobuf version 2 #437

Merged
merged 20 commits into from
May 30, 2022
Merged

Feat/update protobuf version 2 #437

merged 20 commits into from
May 30, 2022

Conversation

seanpearsonuk
Copy link
Collaborator

@seanpearsonuk seanpearsonuk commented May 27, 2022 •
edited
Loading

@seanpearsonuk
protobuf upgrade
bb3edd2
@seanpearsonuk
protobuf upgrade
9cd8c6a
@seanpearsonuk
protobuf upgrade
4f03a88
@seanpearsonuk
protobuf upgrade
1c7431a
@seanpearsonuk
disable settings doc api gen
52c00d8
@seanpearsonuk
disable codegen for doc
6eb5fa4
@seanpearsonuk
disable codegen for doc
3bcaddd
@seanpearsonuk
disable codegen for doc
cf2bc7b
@seanpearsonuk
disable codegen for doc
008da0d
@seanpearsonuk
Revert "disable codegen for doc"
13f7949 
This reverts commit 008da0d.

Revert "disable codegen for doc"

This reverts commit cf2bc7b.

Revert "disable codegen for doc"

This reverts commit 3bcaddd.

Revert "disable codegen for doc"

This reverts commit 6eb5fa4.

Revert "disable settings doc api gen"

This reverts commit 52c00d8.
@seanpearsonuk
use docker image 22r2
6b6ba6c
@seanpearsonuk
use docker image 22r2
e762647
@seanpearsonuk
use docker image 22r2
2583d6e
@seanpearsonuk
use docker image 22r2
e362e71
@seanpearsonuk
use docker image 22r2
b2309ca
@seanpearsonuk
undo docker changes - not working
5e5f2d7
@seanpearsonuk
command_names iter check
e9a9b75
@seanpearsonuk seanpearsonuk marked this pull request as draft May 27, 2022 08:08
@seanpearsonuk seanpearsonuk marked this pull request as ready for review May 27, 2022 10:11
h-krishnan
h-krishnan reviewed May 27, 2022
View reviewed changes
codegen/settingsgen.py Outdated
@@ -64,11 +64,16 @@ def _populate_hash_dict(name, info, cls):
if commands:
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are switching back to 22.2, this change is not required, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No codegen changes should be required if we are switching back to 22.2. I am waiting on that change which was communicated yesterday.

But we should we in general use all of these containers in the codegen defensively?

Copy link
Contributor

Choose a reason for hiding this comment

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

We can consider that in a targeted PR in which other potential issues are also caught.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At the moment I have no idea when we will revert to 22R2 and it's out of my control. We are keen to get everything working again asap.

@seanpearsonuk seanpearsonuk mentioned this pull request May 27, 2022
Closed
h-krishnan
h-krishnan approved these changes May 27, 2022
View reviewed changes
Copy link
Contributor

@h-krishnan h-krishnan left a comment

Choose a reason for hiding this comment

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

I feel that the settings related changes can be skipped.

@mkundu1 mkundu1 merged commit 41ae157 into main May 30, 2022
@mkundu1 mkundu1 deleted the feat/update_protobuf_version_2 branch May 30, 2022 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajain-work ajain-work ajain-work approved these changes

@mkundu1 mkundu1 mkundu1 approved these changes

@h-krishnan h-krishnan h-krishnan approved these changes

@dnwillia-work dnwillia-work Awaiting requested review from dnwillia-work

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@seanpearsonuk @h-krishnan @mkundu1 @ajain-work