- Notifications
You must be signed in to change notification settings - Fork673
feat(ux): display project.name_with_namespace on project repr#1996
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This change the repr from:$ gitlab.projects.get(id=some_id)<Project id:some_id>To:$ gitlab.projects.get(id=some_id)<Project id:some_id name_with_namespace:"group_name / project_name">This is especially useful when working on random projects or listing ofprojects since users generally don't remember projects ids.
Cool. Just as an FYI we also have a |
codecov-commenter commentedApr 29, 2022
Codecov Report
@@ Coverage Diff @@## main #1996 +/- ##==========================================+ Coverage 83.47% 92.51% +9.03%========================================== Files 78 78 Lines 4938 4943 +5 ==========================================+ Hits 4122 4573 +451+ Misses 816 370 -446
Flags with carried forward coverage won't be shown.Click here to find out more.
|
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.
Overall looks good to me. Left a minor suggestion.
Unsure if we want a unit/functional test or not for this?@nejch what do you think?
| if hasattr(self, "name_with_namespace"): | ||
| return ( | ||
| f'{project_repr[:-1]} name_with_namespace:"{self.name_with_namespace}">' |
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.
Can use the!r to quote the value.
| f'{project_repr[:-1]} name_with_namespace:"{self.name_with_namespace}">' | |
| f"{project_repr[:-1]} name_with_namespace:{self.name_with_namespace!r}>" |
So don't tell@nejch 😜 But I still don't use |
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.
Thanks@Psycojoker for this, I really like the idea to make repr more human-readable.
What do you think about generalizing it a bit to have a more declarative approach on individual resources (project, group,..) and handling the__repr__ based on that in theRESTObject method, like we do withid using_id_attr now? Otherwise, we'll have to add a bunch more overrides if people want similar for other objects (or even GroupProject, UserProject.. etc).
For this, I'd introduce a_repr_attr on individual resources, and handle that centrally here:
Lines 160 to 164 in882fe7a
| def__repr__(self)->str: | |
| ifself._id_attr: | |
| returnf"<{self.__class__.__name__}{self._id_attr}:{self.get_id()}>" | |
| else: | |
| returnf"<{self.__class__.__name__}>" |
Just to illustrate this, here's a full list (I think) of objects, a lot of them would benefit from this feature. I don't want you to do this for all of them, but if you could try to add it for every type of*Project we have a good start to check it works. If it's too repetitive, we could introduce mixins for this at some point, or some other more reusable way if you have any ideas :) WDYT@Psycojoker@JohnVillalovos
ApplicationApplicationAppearanceApplicationSettingsAuditEventBroadcastMessageCurrentUserCurrentUserEmailCurrentUserGPGKeyCurrentUserKeyCurrentUserStatusDeployKeyDeployTokenDockerfileEventFeatureGenericPackageGeoNodeGitignoreGitlabciymlGroupGroupAccessRequestGroupAccessTokenGroupAuditEventGroupBadgeGroupBillableMemberGroupBillableMemberMembershipGroupBoardGroupBoardListGroupClusterGroupCustomAttributeGroupDeployTokenGroupDescendantGroupGroupEpicGroupEpicAwardEmojiGroupEpicDiscussionNoteGroupEpicIssueGroupEpicNoteGroupEpicNoteAwardEmojiGroupEpicResourceLabelEventGroupExportGroupHookGroupImportGroupIssueGroupIssuesStatisticsGroupLabelGroupMemberGroupMemberAllGroupMergeRequestGroupMilestoneGroupNotificationSettingsGroupPackageGroupProjectGroupRunnerGroupSubgroupGroupVariableGroupWikiHookIssueIssuesStatisticsKeyLDAPGroupLicenseListMergeRequestNamespaceNotificationSettingsOptionalPagesDomainPersonalAccessTokenProjectProjectAccessRequestProjectAccessTokenProjectAdditionalStatisticsProjectApprovalProjectApprovalRuleProjectArtifactProjectAuditProjectAuditEventProjectBadgeProjectBoardProjectBoardListProjectBranchProjectClusterProjectCommitProjectCommitCommentProjectCommitDiscussionProjectCommitDiscussionNoteProjectCommitStatusProjectCustomAttributeProjectDeployTokenProjectDeploymentProjectDeploymentMergeRequestProjectEnvironmentProjectEventProjectExportProjectFileProjectForkProjectHookProjectImportProjectIssueProjectIssueAwardEmojiProjectIssueDiscussionProjectIssueDiscussionNoteProjectIssueLinkProjectIssueNoteProjectIssueNoteAwardEmojiProjectIssueResourceLabelEventProjectIssueResourceMilestoneEventProjectIssueResourceStateEventProjectIssuesStatisticsProjectJobProjectKeyProjectLabelProjectMemberProjectMemberAllProjectMergeRequestProjectMergeRequestApprovalProjectMergeRequestApprovalRuleProjectMergeRequestApprovalStateProjectMergeRequestAwardEmojiProjectMergeRequestDiffProjectMergeRequestDiscussionProjectMergeRequestDiscussionNoteProjectMergeRequestNoteProjectMergeRequestNoteAwardEmojiProjectMergeRequestPipelineProjectMergeRequestResourceLabelEventProjectMergeRequestResourceMilestoneEventProjectMergeRequestResourceStateEventProjectMergeTrainProjectMilestoneProjectNoteProjectNotificationSettingsProjectPackageProjectPackageFileProjectPagesDomainProjectPipelineProjectPipelineBridgeProjectPipelineJobProjectPipelineScheduleProjectPipelineScheduleVariableProjectPipelineTestReportProjectPipelineTestReportSummaryProjectPipelineVariableProjectProtectedBranchProjectProtectedTagProjectPushRulesProjectRegistryRepositoryProjectRegistryTagProjectReleaseProjectReleaseLinkProjectRemoteMirrorProjectRunnerProjectServiceProjectSnippetProjectSnippetAwardEmojiProjectSnippetDiscussionProjectSnippetDiscussionNoteProjectSnippetNoteProjectSnippetNoteAwardEmojiProjectTagProjectTriggerProjectUserProjectVariableProjectWikiRepositoryMixinRunnerRunnerJobSnippetStarredProjectTodoTopicUserUserActivitiesUserCustomAttributeUserEmailUserEventUserGPGKeyUserImpersonationTokenUserKeyUserMembershipUserPersonalAccessTokenUserProjectUserStatusVariable
About pre-commit - sorry to hear that. Generally the idea of We kind of do need pre-commit or something like that if we keep adding more linters and nitpicky rules though. Otherwise people will push a lot of failing code and it creates more noise for followers and extra effort, especially with new contributors where every push needs approval for CI to run. The other option is that we calm down with the linting a bit and make things looser. But the commit messages at least are nice, as it makes our release process really smooth and we can bring out new versions much quicker if needed :) |
I've started#1998 to address the pre-commit issue. We can also just merge this as is and work on refactoring after, just let us know@Psycojoker if you'd like to try yourself already. :) |
Ok, so let's get this in to have the feature and I'll rework it based on my comment in a follow-up. |
This change the repr from:
To:
This is especially useful when working on random projects or listing of
projects since users generally don't remember projects ids.
On a side note I've encountered this error when trying to do a commit:
I have to admit that, while I totally understand the importance of linters and good commit messages, having a hard dependency on a specific version of nodejs and glibc just do to a commit on a full python feels a bit ... intense? 😅
Anyway, thanks a lot for this project ❤️