Comments (8)
I will revisit this over the next few days and propose a config design/implementation. Will post back here with updates. Thanks!
from protolock.
Hey @viswajithiii -
Before considering the PR you've suggested, would you be open to trying a comment annotation on your RPCs? Currently, the @protolock:skip
hint will ignore a message from being saved to the lock file, and thus is ignored when comparing versions.
Here's an example:
Lines 55 to 56 in c6b4295
(here, the message NextRequest {}
is not added to the lock file at all).
The comment (called a "hint" in protolock) is not implemented on any other type aside from message
, and I think this would be a better PR, at least initially, since it may satisfy your need, and still abide by the existing use of the hint in protolock today. It would be great if a PR would cover the implementation of these hints on all the supported types.
My main concern with a configurable base of built-in rules, is that depending on how the configuration is stated, a user may not be aware of ignored rules, and it could break API compatibility.
I see value in making the tool more flexible, though, and appreciate this idea!
from protolock.
Hey @nilslice, thanks for your response! Thanks for the suggestion about @protolock: skip
-- I wasn't aware of this piece of functionality, and it's helpful. However, it doesn't solve this specific problem -- I think having to put this annotation above every RPC would be inconvenient and unnecessarily pollute our proto files.
Further, this is not the only use case I have in mind for this. Another example of a thing I would like to do is to have a rule that doesn't require field names to be reserved, just ids. I'm not sure there's a great way to achieve this with the tool right now.
I do understand your concerns about this creating a whole new surface of API compatibility that would have to be preserved. For now, I've created a fork and made the changes I need in a (destructive) way and I'm using that for now. I'll try to spend some time thinking of how best to achieve this within the constraints of this repo, though.
from protolock.
@viswajithiii - I wanted to follow up on this, since I am starting to think about supporting an optional configuration file for protolock. Within this conf, it would be possible to disable any of the built-in rules (among other features).
I was wondering if you had come up with a solution for your initial issue or if you have some feedback/suggestions for a conf file? Thanks!
See #86 for some more info as this progresses.
from protolock.
Would also be interested in being able to enable/disable rules by name.
Use case:
We represent schemas for analytics data sent as JSON, with proto schemas. Since analytics event data must maintain backwards compatibility with historical events, removing fields, even when marking them as reserved is not allowed. If a field needs to be removed, then really it should be in an entirely new event in our system.
I'd like to disable the No Removing Fields Without Reserve
check and replace it by introducing a plugin of my own which just says "no removing new fields, period", so my users are not presented with two (slightly contradictory) error messages.
But right now the only way to accomplish that is to wrap protolock
and expressly filter out any undesired error messages with a hacky regex, and also use my custom plugin.
from protolock.
@nilslice I apologize, I completely missed your previous note, but got notified now that @wadejensen commented. An optional config file sounds like a great way to go about this -- one that is flexible, and doesn't break backwards compatibility. I'd be happy to share my thoughts on any design you come up with, if that would be helpful. 🙂
from protolock.
@nilslice any updates?
from protolock.
@guyisra I suppose a minimal, extensible option would be to read a --config
path and only support a single config option for the time being...
# protolock.yaml
skip_rules:
- NoUsingReservedFields
- NoRemovingReservedFields
And provide this config to the rule checking step (or create a CompareWithConfig
func?), which has knowledge of the name of each rule as it is being run:
Lines 474 to 492 in c61398f
I don't have the time currently to implement and test, but would be happy to review a PR if you need this functionality and would like to take a stab it it!
from protolock.
Related Issues (20)
- Error building protolock on ppc64le HOT 2
- Processing a single file HOT 3
- Enum aliases not supported HOT 3
- Does protolock support proto2 ? HOT 1
- Propose to publish ARM64 binary in releases HOT 6
- Convert circleci jobs to GitHub Actions
- oneof structure is not persisted in protolock HOT 4
- Options not parsed at the Enum level HOT 2
- found "stream" but expected [rpc method] for a valid proto HOT 3
- Add artefacts to Maven repository HOT 7
- Update docker hub image HOT 3
- Allow ability to include proto files that are not in the root tree
- TestOrder consistently fails
- Nit: typo in info string for protolock cmd HOT 1
- [.gitignore] Remove "protolock" line or edit file to not filter out "cmd/protolock" HOT 1
- False positive in camelCase naming violation for EnumValueOptions builtin HOT 1
- Embedded comments confuse the parser HOT 7
- Deeply nested messages may conflict HOT 2
- Duplicate message names with different fields are considered valid HOT 3
- protolock status shows a lot of warnings
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from protolock.