APIv3 endpoint to manage Environment Variables #5913
Conversation
|
@stsewd check it now ;) |
| 'modified', | ||
| 'name', | ||
| 'value', | ||
| 'project', |
There was a problem hiding this comment.
do we really need the project here? (haven't seen the other endpoints)
There was a problem hiding this comment.
We should just make sure it's the same everywhere, unless we have a good reason (and a code comment)
There was a problem hiding this comment.
The project is the main relation with the object(s). I think it's good to have it here. All the other responses also relates with the project.
Actually, VersionSerializer is the only one that does not returns project. I think I should add it there as well to keep consistency.
| _self = serializers.SerializerMethodField() | ||
| project = serializers.SerializerMethodField() | ||
|
|
||
| def get__self(self, obj): |
There was a problem hiding this comment.
2 underscsores is weird here.
There was a problem hiding this comment.
Agree. This is because we are using _self as our attribute since it's kind of a "special name". Then, we need get_ and the attribute...
| 'modified', | ||
| 'name', | ||
| 'value', | ||
| 'project', |
There was a problem hiding this comment.
We should just make sure it's the same everywhere, unless we have a good reason (and a code comment)
| @@ -1483,6 +1483,8 @@ class EnvironmentVariable(TimeStampedModel, models.Model): | |||
| help_text=_('Project where this variable will be used'), | |||
| ) | |||
|
|
|||
| objects = RelatedProjectQuerySet.as_manager() | |||
There was a problem hiding this comment.
Did we not need this before? We only appear to be calling all on it.
There was a problem hiding this comment.
Probably we needed it, but nothing was using it to list these related objects.
I agree that it's long, but it's not a bad name to me. |
valueis not shownThis PR is based on #5911 (so, maybe you want to review that one first ;) )