-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Go: use package grouping in existing models-as-data models #16992
Conversation
Click to show differences in coveragegoGenerated file changes for go
- `Go JOSE <https://github.com/go-jose/go-jose>`_,"``github.com/go-jose/go-jose*``, ``github.com/square/go-jose*``, ``gopkg.in/square/go-jose*``",,8,2
+ `Go JOSE <https://github.com/go-jose/go-jose>`_,"``github.com/go-jose/go-jose*``, ``github.com/square/go-jose*``, ``gopkg.in/square/go-jose*``",,12,3
- Totals,,204,898,24
+ Totals,,204,902,25
+ github.com/square/go-jose/jwt,1,,4,,1,,4,
- gopkg.in/square/go-jose.v2/jwt,1,,4,,1,,4,
+ gopkg.in/square/go-jose/jwt,1,,4,,1,,4, |
abf4be8
to
0f7e1ef
Compare
0f7e1ef
to
4c3220e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I am wondering about the (need for) the last commit though. For the purpose of those tests, does it really matter which line they come from or could the name of the file or some other, constant information suffice?
I'm not sure I totally follow. The last commit doesn't relate to line numbers. Currently, many tests include provenance information for edges which uses a different number for each models-as-data model. Adding or removing models perturbs these numbers, and causes a lot of unrelated test changes. There are plans in motion to fix this soon, but in the meantime we are just having to work around the constant unrelated test churn that makes all models-related tests conflict with each other. |
Sorry, I had just assumed that they were line numbers rather than something else. That doesn't change my question though, but you have answered it anyway that there are plans to change this. |
Use the mechanism introduced in #16941 in the existing models.