Project

Profile

Help

Issue #1442

Make RepoURLModifier more IDE-friendly

Added by semyers almost 5 years ago. Updated over 1 year ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
Low
Assignee:
Sprint/Milestone:
-
Start date:
Due date:
Estimated time:
Severity:
1. Low
Version:
Master
Platform Release:
2.8.0
OS:
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
Easy Fix, Pulp 2
Sprint:
Quarter:

Description

(and also more developer-friendly)

Currently, in pulp_rpm[0], the RepoURLModifier stashes the passed-in args in a dict as the conf instance attr. This is an artifact from earlier incarnations of that class, and serves no practical purpose. Instead of adding those to a dict, they should be normal instance attributes.

The conf dict keys are used for validation in __call__, but I think it's probably reasonable to get rid of the **kwargs usage in that method to support more IDE- and developer-friendliness; it looks less DRY, but ends up being more explicit and readable, which is probably an overall win.

[0]: https://github.com/pulp/pulp_rpm/blob/0d3a483c905e9c54ebd1ff01ad814fa38f8fb6bb/plugins/pulp_rpm/plugins/importers/yum/utils.py#L105

Associated revisions

Revision 88881cbf View on GitHub
Added by semyers almost 5 years ago

RepoURLModifier is more IDE- and user-friendly

In addition to explicitly declaring the kwargs to __init__ and __call__, I took a step back and considered if there was any value to storing path_append and ensure_trailing_slash as stateful defaults. Based on the current usage and my best judgement, there was no value in that, and in fact having the same signature for both methods only made things more confusing. Hopefully the stateful nature of of this class is more clear, so in addition to being IDE-friendly, it's much easier for humans to grok.

https://pulp.plan.io/issues/1442 fixes #1442

History

#1 Updated by mhrivnak almost 5 years ago

  • Triaged changed from No to Yes

#2 Updated by semyers almost 5 years ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to semyers

#3 Updated by semyers almost 5 years ago

  • Status changed from ASSIGNED to POST
  • Version set to Master

#4 Updated by semyers almost 5 years ago

  • Status changed from POST to MODIFIED
  • % Done changed from 0 to 100

#5 Updated by rbarlow almost 5 years ago

  • Platform Release set to 2.8.0

#6 Updated by dkliban@redhat.com almost 5 years ago

  • Status changed from MODIFIED to 5

#7 Updated by dkliban@redhat.com over 4 years ago

  • Status changed from 5 to CLOSED - CURRENTRELEASE

#8 Updated by bmbouter over 1 year ago

  • Tags Pulp 2 added

Please register to edit this issue

Also available in: Atom PDF