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

api reference tweaks #651

Merged
merged 9 commits into from
Sep 15, 2021

Conversation

shadeofblue
Copy link
Contributor

No description provided.

@shadeofblue shadeofblue requested review from a team and johny-b September 14, 2021 08:37
Copy link
Contributor

@johny-b johny-b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some minor comments, but I really like the result, that's what I was hoping for : )

I wouldn't deal now with things like duplicated handbook url in the index, there's a task for this: https://github.com/orgs/golemfactory/projects/14

yapapi/golem.py Outdated Show resolved Hide resolved
yapapi/payload/__init__.py Outdated Show resolved Hide resolved
@@ -2,6 +2,7 @@
import itertools
from typing import Any, Awaitable, Callable, Dict, Iterator, Optional, List, TYPE_CHECKING

import yapapi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duh, this is ugly.

I more-or-less understand why we have this, but I think the generated documentation is exactly the same before this change, so this only silences warnings. I don't think it's worth it?
Or maybe if Script had :params: section this would be different?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, it's not exactly the same -> without it, the type hints don't link and there's an ugly ForwardRef link instead...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

anyway, we definitely do have a differing definition of "ugly" ... to me, it's "inelegant" at most ...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I tried this once and compared result files with diff and got no difference. I'll try this again
  2. OK, inelegant is fine (although I'd say everything related to python annotations deserves this name). Either way, I'd be glad to get rid of it if we can : )

@johny-b johny-b merged commit 8cd8aaa into johny-b/585-api-reference-sphinx Sep 15, 2021
@shadeofblue shadeofblue deleted the blue/api-reference-tweaks branch September 16, 2021 11:23
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