Skip to content
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

Fix name resolution for intersphinx links #126

Merged
merged 3 commits into from Sep 13, 2016

Conversation

adiroiban
Copy link
Member

@adiroiban adiroiban commented Aug 3, 2016

See #124

The root problem is Documentable.expandName but I was not able to fix its implementation.
See #125 for more details.

I went with a dirty hack.

I have also increased the release number, so once this is merged we can create a tag and release a new version.

@codecov-io
Copy link

codecov-io commented Aug 3, 2016

Current coverage is 60.36% (diff: 100%)

Merging #126 into master will increase coverage by 0.02%

@@             master       #126   diff @@
==========================================
  Files            20         20          
  Lines          3293       3295     +2   
  Methods           0          0          
  Messages          0          0          
  Branches        700        701     +1   
==========================================
+ Hits           1987       1989     +2   
  Misses         1105       1105          
  Partials        201        201          

Powered by Codecov. Last update 4329634...9d69400

@glyph
Copy link
Member

glyph commented Sep 6, 2016

@adiroiban - I want to say "add a test case for this" but somehow this just increases coverage already? :)

@adiroiban
Copy link
Member Author

I start writing unit tests for Documentable.expandName ... but I found out that I had no idea what is going on there and how to write a proper fix and proper tests for it.

with a previous branch I have added functional end-to-end tests by running pydoctor against Twisted trunk...so we do have tests for this change... just that they are not unit tests and not easy to follow.

so, I can write a unit tests for translate_identifier_xref ... but the problem is that I don't really know what is wrong here and how to write the docstring for the test

@glyph
Copy link
Member

glyph commented Sep 6, 2016

@mwhudson - any help?

@hawkowl
Copy link
Member

hawkowl commented Sep 13, 2016

Since this has 100% coverage, and it fixes an issue, and we don't really know any better way than yoloing it... sure, let's go with this.

@hawkowl hawkowl merged commit 6ed85ea into master Sep 13, 2016
@hawkowl hawkowl deleted the intersphinx-not-so-fuller-id branch September 13, 2016 16:22
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.

None yet

4 participants