Conversation
|
Will need to update |
| --add-excluded-prefixes ./envoy/ ./envoy_build_config/extensions_build_config.bzl ./WORKSPACE ./dist/Envoy.framework/ \ | ||
| --skip_envoy_build_rule_check "$ENVOY_FORMAT_ACTION" | ||
| --skip_envoy_build_rule_check "$ENVOY_FORMAT_ACTION" \ | ||
| --namespace_check_excluded_paths ./examples/ ./library/java/ |
There was a problem hiding this comment.
I'll remove the NOLINTs that are no longer necessary for Objective-C in another PR.
There was a problem hiding this comment.
I'd suggest including that in this PR - then we can see that it's working via CI, right?
There was a problem hiding this comment.
ah nevermind, for some reason I was thinking this was the objective C PR.
There was a problem hiding this comment.
Yea the diff as is will validate that this is working since prior to the upstream Envoy change, we were getting NOLINT violations for the .java files
| AccessModifierOffset: -2 | ||
| ColumnLimit: 100 | ||
| DerivePointerAlignment: false | ||
| IndentWidth: 2 |
There was a problem hiding this comment.
I don’t have a preference as to what this value is, but all the other languages (and upstream) use 2 🤷🏽♂️
mattklein123
left a comment
There was a problem hiding this comment.
Awesome. Your changes are merged upstream so just needs a submodule update.
|
Updated with upstream sha @mattklein123 @goaway |
`clang-format` can be used for auto-formatting Java files the same way it is for Objective-C and C++. 2 spaces were selected for spacing to maintain parity with upstream. Depends on: envoyproxy/envoy#7210 envoyproxy/envoy#7211 Signed-off-by: Michael Rebello <mrebello@lyft.com> update envoy sha Signed-off-by: Michael Rebello <mrebello@lyft.com> reformat after rebasing Signed-off-by: Michael Rebello <mrebello@lyft.com>
Signed-off-by: Michael Rebello <mrebello@lyft.com>
mattklein123
left a comment
There was a problem hiding this comment.
Awesome!
@rebello95 friendly request to not force push if possible in the future. It makes reviews more difficult.
|
Sounds good, thanks @mattklein123. |
These are obsoleted by this change: #56 (comment) Signed-off-by: Michael Rebello <mrebello@lyft.com>
These are obsoleted by this change: #56 (comment) Signed-off-by: Michael Rebello <mrebello@lyft.com>
This was fixed as part of #56. Signed-off-by: Michael Rebello <mrebello@lyft.com>
This was fixed as part of #56. Signed-off-by: Michael Rebello <mrebello@lyft.com>
These are obsoleted by this change: envoyproxy/envoy-mobile#56 (comment) Signed-off-by: Michael Rebello <mrebello@lyft.com> Signed-off-by: JP Simard <jp@jpsim.com>
This was fixed as part of envoyproxy/envoy-mobile#56. Signed-off-by: Michael Rebello <mrebello@lyft.com> Signed-off-by: JP Simard <jp@jpsim.com>
These are obsoleted by this change: envoyproxy/envoy-mobile#56 (comment) Signed-off-by: Michael Rebello <mrebello@lyft.com> Signed-off-by: JP Simard <jp@jpsim.com>
This was fixed as part of envoyproxy/envoy-mobile#56. Signed-off-by: Michael Rebello <mrebello@lyft.com> Signed-off-by: JP Simard <jp@jpsim.com>
clang-formatcan be used for auto-formatting Java files the same way it is for Objective-C and C++.2 spaces were selected for spacing to maintain parity with upstream.
Depends on:
envoyproxy/envoy#7210
envoyproxy/envoy#7211
#55
Signed-off-by: Michael Rebello mrebello@lyft.com