Skip to content

stubsabot: add project links #8158

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.

Sign up for GitHub

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

Merged
JelleZijlstra merged 1 commit into from
Jun 25, 2022
Merged

stubsabot: add project links #8158

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions scripts/stubsabot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class PypiInfo:
# https://warehouse.pypa.io/api-reference/json.html#get--pypi--project_name--json
# Corresponds to a single entry from `releases` for the given version
release_to_download: dict[str, Any]
info: dict[str, Any]


async def fetch_pypi_info(distribution: str, session: aiohttp.ClientSession) -> PypiInfo:
Expand All @@ -82,6 +83,7 @@ async def fetch_pypi_info(distribution: str, session: aiohttp.ClientSession) ->
version=packaging.version.Version(version),
upload_date=date,
release_to_download=release_to_download,
info=j["info"],
)


Expand All @@ -91,6 +93,7 @@ class Update:
stub_path: Path
old_version_spec: str
new_version_spec: str
links: dict[str, str]

def __str__(self) -> str:
return f"Updating {self.distribution} from {self.old_version_spec!r} to {self.new_version_spec!r}"
Expand All @@ -102,6 +105,7 @@ class Obsolete:
stub_path: Path
obsolete_since_version: str
obsolete_since_date: datetime.datetime
links: dict[str, str]

def __str__(self) -> str:
return f"Marking {self.distribution} as obsolete since {self.obsolete_since_version!r}"
Expand Down Expand Up @@ -173,19 +177,29 @@ async def determine_action(stub_path: Path, session: aiohttp.ClientSession) -> U
if pypi_info.version in spec:
return NoUpdate(stub_info.distribution, "up to date")

project_urls = pypi_info.info["project_urls"] or {}
maybe_links: dict[str, str | None] = {
"Release": pypi_info.info["release_url"],
"Homepage": project_urls.get("Homepage"),
"Changelog": project_urls.get("Changelog") or project_urls.get("Changes") or project_urls.get("Change Log"),
}
links = {k: v for k, v in maybe_links.items() if v is not None}

if await package_contains_py_typed(pypi_info.release_to_download, session):
return Obsolete(
stub_info.distribution,
stub_path,
obsolete_since_version=str(pypi_info.version),
obsolete_since_date=pypi_info.upload_date,
links=links,
)

return Update(
distribution=stub_info.distribution,
stub_path=stub_path,
old_version_spec=stub_info.version_spec,
new_version_spec=get_updated_version_spec(stub_info.version_spec, pypi_info.version),
links=links,
)


Expand Down Expand Up @@ -268,7 +282,9 @@ async def suggest_typeshed_update(update: Update, session: aiohttp.ClientSession
return
subprocess.check_call(["git", "push", "origin", branch_name, "--force-with-lease"])

body = """\
body = "\n".join(f"{k}: {v}" for k, v in update.links.items())
body += """

If stubtest fails for this PR:
- Leave this PR open (as a reminder, and to prevent stubsabot from opening another PR)
- Fix stubtest failures in another PR, then close this PR
Copy link
Collaborator

@Akuli Akuli Jun 26, 2022

Choose a reason for hiding this comment

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

I have never understood these instructions. Why not just fix the PR by pushing commits directly to it, and merge when stubtest passes again?

Copy link
Member

@AlexWaygood AlexWaygood Jun 26, 2022

Choose a reason for hiding this comment

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

Yes, that seems to be the more common rhythm we're settling into.

Copy link
Collaborator Author

@hauntsaninja hauntsaninja Jun 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

It's a good question. It's because of what the current code will do if we run it automatically on a regular basis (rather than me running it manually in batches). stubsabot.py just makes a commit off of master and pushes it. master changes regularly, so these pushes would fail. Rather than failing, silently doing nothing, or some more complicated logic that checks what's on the remote, it currently force pushes to the remote:

typeshed/scripts/stubsabot.py

Line 283 in a9fb8c5

subprocess.check_call(["git", "push", "origin", branch_name, "--force-with-lease"])

So this means it could undo changes we made.

But thinking about it a little bit more, silently doing nothing might not be that bad an option?

The other reason for these instructions is in theory non-maintainers could contribute (and they might benefit from instructions more so than maintainers).

Expand All @@ -295,7 +311,8 @@ async def suggest_typeshed_obsolete(obsolete: Obsolete, session: aiohttp.ClientS
return
subprocess.check_call(["git", "push", "origin", branch_name, "--force-with-lease"])

await create_or_update_pull_request(title=title, body="", branch_name=branch_name, session=session)
body = "\n".join(f"{k}: {v}" for k, v in obsolete.links.items())
await create_or_update_pull_request(title=title, body=body, branch_name=branch_name, session=session)


async def main() -> None:
Expand Down