anza-xyz / move

Move compiler targeting llvm supported backends
https://discord.gg/wFgfjG9J
Apache License 2.0
109 stars 34 forks source link

ci: remove add-pr-link related actions #353

Closed yihau closed 1 year ago

yihau commented 1 year ago

Problem

We added add-pr-link for tracking commits. Actually we can find the PR number in the web page

Screenshot 2023-09-09 at 2 18 27 PM

or if we have setup gh, we can use this command to find it as well

gh pr list --search <SHA> --state merged --repo solana-labs/move

I guess this action is a little bit trivial atm. (also a little bit aggressive cuz we just modify the original PR with a third party)

Summary of Changes

Remove all add-pr-link related code.

yihau commented 1 year ago

@ksolana this one is just a draft. we can discuss more if you have any concerns!

ksolana commented 1 year ago

Finding PR on webpage is not difficult. The problem is when we do git log it doesn't show the link to PR where it was merged from. That poses a difficulty for developers.

For example, this is what we currently get with git log:

commit d7af40d72444a742ed3faca80e8434dc839af67f (HEAD -> doc, origin/llvm-sys)
Author: ksolana <110843012+ksolana@users.noreply.github.com>
Date:   Sun Sep 10 12:11:11 2023 -0700

    Add doc on running move-stdlib and framework tests

commit c0a5464d749361d89c83f3a33090bc4c497e0b42
Author: Dmitri Makarov <dmakarov@alumni.stanford.edu>
Date:   Fri Sep 8 11:45:03 2023 -0400

    Enable remaining move-cli build tests for Solana backend

commit 763d827036f330861d784e62e1867cc3c238d76a
Author: Dmitri Makarov <dmakarov@alumni.stanford.edu>
Date:   Fri Sep 8 20:26:11 2023 -0400

    Fix clippy errors

commit d7f53a8b084582f75546ed8b9bdf9279e837c4e5
Author: Dmitri Makarov <dmakarov@alumni.stanford.edu>
Date:   Fri Sep 8 20:25:56 2023 -0400

    Bump solana_rbpf to 0.7.1

    This entails bumping the Rust used to build the Move crates to version
    1.72.0, and bumping solana-* dependencies revision.

commit 4526ac24ddc42b2ddf4e2d8794b38f6e9fc1ef48
Author: Dmitri Makarov <dmakarov@alumni.stanford.edu>
Date:   Fri Sep 8 11:55:42 2023 -0400

    Enable remaining move-cli move_unit_test tests for Solana backend

Instead if we got something like:

commit d7af40d72444a742ed3faca80e8434dc839af67f (HEAD -> doc, origin/llvm-sys)
Author: ksolana <110843012+ksolana@users.noreply.github.com>
Date:   Sun Sep 10 12:11:11 2023 -0700

    Add doc on running move-stdlib and framework tests
    Differential Revision: https://github.com/solana-labs/move/pull/354

commit c0a5464d749361d89c83f3a33090bc4c497e0b42
Author: Dmitri Makarov <dmakarov@alumni.stanford.edu>
Date:   Fri Sep 8 11:45:03 2023 -0400

    Enable remaining move-cli build tests for Solana backend
    Differential Revision: https://github.com/solana-labs/move/pull/350

commit 763d827036f330861d784e62e1867cc3c238d76a
Author: Dmitri Makarov <dmakarov@alumni.stanford.edu>
Date:   Fri Sep 8 20:26:11 2023 -0400

    Fix clippy errors

    Differential Revision: https://github.com/solana-labs/move/pull/351

commit d7f53a8b084582f75546ed8b9bdf9279e837c4e5
Author: Dmitri Makarov <dmakarov@alumni.stanford.edu>
Date:   Fri Sep 8 20:25:56 2023 -0400

    Bump solana_rbpf to 0.7.1

    This entails bumping the Rust used to build the Move crates to version
    1.72.0, and bumping solana-* dependencies revision.

    Differential Revision: https://github.com/solana-labs/move/pull/351

commit 4526ac24ddc42b2ddf4e2d8794b38f6e9fc1ef48
Author: Dmitri Makarov <dmakarov@alumni.stanford.edu>
Date:   Fri Sep 8 11:55:42 2023 -0400

    Enable remaining move-cli move_unit_test tests for Solana backend
    Differential Revision: https://github.com/solana-labs/move/pull/350
yihau commented 1 year ago

how do you feel about this workflow: (I guess the PR number is only valuable when we care about a specific commit)

  1. we git log and search something
  2. we find a commit which we are interested
  3. use gh pr list --search <SHA> --state merged --repo solana-labs/move to get the PR number

I still feel it's not a very good idea to modify all commits to a PR.

ksolana commented 1 year ago

yeah it's a weird situation. i feel like if there was a way to get a PR number before creating a pull request, we could patch the commit message with PR link without adding a new commit.

Given the status quo, I'll let you chose the workflow that makes most sense.

yihau commented 1 year ago

the PR number is incremental. if you want to keep a number, I guess you need to create it first or someone will take the number.

or maybe we can have a script which looks like

echo "https://github.com/solana-labs/move/pull/$(gh pr list --search <SHA> --state merged --repo solana-labs/move --json number | jq '.[0].number')"

it can print the pr links for us as well. (the only requirement is to setup gh cli)

ksolana commented 1 year ago

the PR number is incremental. if you want to keep a number, I guess you need to create it first or someone will take the number.

or maybe we can have a script which looks like

echo "https://github.com/solana-labs/move/pull/$(gh pr list --search <SHA> --state merged --repo solana-labs/move --json number | jq '.[0].number')"

it can print the pr links for us as well. (the only requirement is to setup gh cli)

What if two people run this command at the same time

yihau commented 1 year ago

What if two people run this command at the same time

This command is used to query a PR link for an existing commit. I think it is totally fine to query for the same commit at the same time.