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

Removes all the occurences of omitempty #2209

Merged
merged 2 commits into from
Mar 19, 2020
Merged

Removes all the occurences of omitempty #2209

merged 2 commits into from
Mar 19, 2020

Conversation

jaybatra26
Copy link
Contributor

@jaybatra26 jaybatra26 commented Mar 4, 2020 •
edited by pracucci
Loading

What this PR does:
This PR removes all the occurences of omitempty
from structs.

Signed-off by Jay Batra [email protected]

Which issue(s) this PR fixes:
Fixes #2173

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

pracucci
pracucci reviewed Mar 4, 2020
View reviewed changes
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Thanks @jaybatra26 for working on it. So, we should just remove omitempty in the Cortex config context. This means that you should rollback any change to:

  • vendor/
  • *.proto
  • *.pb.go

@pracucci
Copy link
Contributor

pracucci commented Mar 4, 2020

Could you also rebase master and sign your commit(s), please?

@pstibrany
Copy link
Contributor

pstibrany commented Mar 5, 2020 •
edited
Loading

Please don't change *.pb.go files, without changing corresponding .proto files. .pb.go files are generated from .proto.

Update: see what Marco said.

@jaybatra26
Copy link
Contributor Author

new_config.txt
old_config.txt
@pracucci PTAL at above files for comparison of configurations.

@jaybatra26
Copy link
Contributor Author

@pracucci Is there a need to add tests as well to validate the change in config?

@jaybatra26 jaybatra26 changed the title WIP: Removes all the occurences of omitempty Removes all the occurences of omitempty Mar 15, 2020
bboreham
bboreham reviewed Mar 18, 2020
View reviewed changes
Copy link
Contributor

@bboreham bboreham left a comment

Choose a reason for hiding this comment

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

Very good; I have a couple of comments though.

CHANGELOG.md Outdated Show resolved Hide resolved
pkg/ring/lifecycler.go Outdated Show resolved Hide resolved
@jaybatra26
Removes all the occurences of omitempty(cortexproject#2209)
d6b1fcd 
This PR removes all the occurences of omitempty
from structs.

Signed-off by Jay Batra <[email protected]>

Signed-off-by: Jay Batra <[email protected]>
@jaybatra26
Copy link
Contributor Author

@bboreham Done

pracucci
pracucci approved these changes Mar 18, 2020
View reviewed changes
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

CHANGELOG.md Outdated Show resolved Hide resolved
gouthamve
gouthamve approved these changes Mar 18, 2020
View reviewed changes
Copy link
Contributor

@gouthamve gouthamve left a comment

Choose a reason for hiding this comment

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

Nice work!

@pracucci pracucci merged commit 51f7047 into cortexproject:master Mar 19, 2020
pracucci added a commit to grafana/cortex that referenced this pull request Mar 19, 2020
@pracucci
Squashed commit of the following:
5ee1e6e 
commit 8e9c601
Author: Marco Pracucci <[email protected]>
Date:   Thu Mar 19 10:44:34 2020 +0100

    Avoid a superfluous querier integration test

    Signed-off-by: Marco Pracucci <[email protected]>

commit 9691dc6
Author: Marco Pracucci <[email protected]>
Date:   Thu Mar 19 09:26:30 2020 +0100

    Separated metrics helpers unit tests using go blocks

    Signed-off-by: Marco Pracucci <[email protected]>

commit d32afe8
Author: Marco Pracucci <[email protected]>
Date:   Thu Mar 19 09:23:34 2020 +0100

    Reworded util.StringsContain() comment and params

    Signed-off-by: Marco Pracucci <[email protected]>

commit a5000a8
Author: Marco Pracucci <[email protected]>
Date:   Thu Mar 19 09:21:54 2020 +0100

    Update docs/operations/dns-service-discovery.md

    Signed-off-by: Marco Pracucci <[email protected]>

    Co-Authored-By: Peter Štibraný <[email protected]>

commit 1babd96
Author: Marco Pracucci <[email protected]>
Date:   Thu Mar 19 09:21:37 2020 +0100

    Update integration/e2e/service.go

    Signed-off-by: Marco Pracucci <[email protected]>

    Co-Authored-By: Peter Štibraný <[email protected]>

commit c1ed809
Author: Marco Pracucci <[email protected]>
Date:   Thu Mar 19 09:01:57 2020 +0100

    Added a timeout to the 'docker inspect' command to avoid the command being stuck

    Signed-off-by: Marco Pracucci <[email protected]>

commit 271fb8d
Author: Marco Pracucci <[email protected]>
Date:   Thu Mar 19 08:52:09 2020 +0100

    Updated doc

    Signed-off-by: Marco Pracucci <[email protected]>

commit a922d32
Author: Marco Pracucci <[email protected]>
Date:   Thu Mar 19 08:43:20 2020 +0100

    Fixed unit tests

    Signed-off-by: Marco Pracucci <[email protected]>

commit 50398ab
Author: Marco Pracucci <[email protected]>
Date:   Thu Mar 19 08:40:40 2020 +0100

    Fixed doc

    Signed-off-by: Marco Pracucci <[email protected]>

commit f077389
Author: Marco Pracucci <[email protected]>
Date:   Wed Mar 18 18:57:52 2020 +0100

    Documented blocks storage index cache backends

    Signed-off-by: Marco Pracucci <[email protected]>

commit 330e449
Author: Marco Pracucci <[email protected]>
Date:   Wed Mar 18 18:14:12 2020 +0100

    Updated CHANGELOG

    Signed-off-by: Marco Pracucci <[email protected]>

commit 0233519
Author: Marco Pracucci <[email protected]>
Date:   Thu Mar 12 17:40:21 2020 +0100

    Added memcached support to the blocks storage index cache

    Signed-off-by: Marco Pracucci <[email protected]>

commit 305b16c
Author: Marco Pracucci <[email protected]>
Date:   Thu Mar 19 10:46:43 2020 +0100

    Introduced a basic logger to the integration tests framework (cortexproject#2293)

    * Introduced a basic logger to the integration tests framework

    Signed-off-by: Marco Pracucci <[email protected]>

    * Fixed linter

    Signed-off-by: Marco Pracucci <[email protected]>

    * Increased integration tests timeout in CI

    Signed-off-by: Marco Pracucci <[email protected]>

commit 51f7047
Author: Jay Batra <[email protected]>
Date:   Thu Mar 19 13:19:39 2020 +0530

    Removes all the occurences of omitempty (cortexproject#2209)

    * Removes all the occurences of omitempty(cortexproject#2209)

    This PR removes all the occurences of omitempty
    from structs.

    Signed-off by Jay Batra <[email protected]>

    Signed-off-by: Jay Batra <[email protected]>

    * Update CHANGELOG.md

    Co-authored-by: Marco Pracucci <[email protected]>

commit bf59f9f
Author: Sandeep Sukhani <[email protected]>
Date:   Wed Mar 18 17:00:11 2020 +0530

    support for cassandra in integration tests (cortexproject#2275)

    * support for cassandra in integration tests

    Signed-off-by: Sandeep Sukhani <[email protected]>

    * changes suggested from PR review

    Signed-off-by: Sandeep Sukhani <[email protected]>

    * using ConcreteService.Endpoint for getting http endpoint for readiness probe

    Signed-off-by: Sandeep Sukhani <[email protected]>

commit 87fa252
Author: Rajat Vig <[email protected]>
Date:   Wed Mar 18 08:49:45 2020 +0000

    Fix the import for test for bucket_client (cortexproject#2285)

    Signed-off-by: Rajat Vig <[email protected]>
@jaybatra26
Copy link
Contributor Author

Thanks everyone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bboreham bboreham bboreham left review comments

@pstibrany pstibrany pstibrany approved these changes

@pracucci pracucci pracucci approved these changes

@gouthamve gouthamve gouthamve approved these changes

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove "omitempty" from exported config options
5 participants
@jaybatra26 @pracucci @pstibrany @gouthamve @bboreham