Skip to content

dependency: harmonize string representation#393

Merged
neersighted merged 1 commit intopython-poetry:mainfrom
radoering:harmonize-dependency-str-representation
Jun 6, 2022
Merged

dependency: harmonize string representation#393
neersighted merged 1 commit intopython-poetry:mainfrom
radoering:harmonize-dependency-str-representation

Conversation

@radoering
Copy link
Member

  • Added tests for changed code.
  • Updated documentation for changed code.

In #103 Dependency.__str__ has been changed to return base_pep_508_name. Each sub class of Dependency still has its own (inconsistent) __str__ implementation. For example extras are not included, which can be confusing/annoying. Further, URLDependency does not even include the url but only hard-coded "url".

I was thinking about only returning base_pep_508_name, but that does not include the constraint for direct origin dependencies and thus makes debugging more difficult. Since __str__ is especially used in (debug) prints, it should be ok not to return a PEP 508 compliant string. (At least, we didn't do that before.) Thus, base_pep_508_name enriched with constraint is returned. That way, we can avoid multiple implementations and still have all relevant information.

@radoering radoering force-pushed the harmonize-dependency-str-representation branch from a5c1144 to 9123a8e Compare June 5, 2022 16:40
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 6, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.8% 0.8% Duplication

@neersighted neersighted merged commit 2bfcbe4 into python-poetry:main Jun 6, 2022
@radoering radoering mentioned this pull request Jul 9, 2022
@radoering radoering deleted the harmonize-dependency-str-representation branch November 24, 2024 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants