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

Consistent named imports for RHP packages #1678

Open
ChrisSchinnerl opened this issue Nov 18, 2024 · 2 comments
Open

Consistent named imports for RHP packages #1678

ChrisSchinnerl opened this issue Nov 18, 2024 · 2 comments
Labels
good first issue Good for newcomers
Milestone

Comments

@ChrisSchinnerl
Copy link
Member

Right now we use rhp, rhpvX, rhpX and potentially other names pretty inconsistently for importing the paths
go.sia.tech/core/rhp/vX, go.sia.tech/coreutils/rhp/vX and go.sia.tech/renterd/internal/rhp/vX.

We should correct that to be consistent about the name each import gets. I think outside of renterd we are mostly consistent so it's a good idea to check the other repos first and reuse the conventions if possible.

@ChrisSchinnerl ChrisSchinnerl converted this from a draft issue Nov 18, 2024
@ChrisSchinnerl ChrisSchinnerl moved this from Triage to Backlog in Sia Nov 18, 2024
@ChrisSchinnerl ChrisSchinnerl added the good first issue Good for newcomers label Nov 18, 2024
@ChrisSchinnerl ChrisSchinnerl added this to the v2.1.0 milestone Nov 18, 2024
@ahuangg
Copy link

ahuangg commented Dec 22, 2024

Hi @ChrisSchinnerl I'm interested in this issue, I scanned through other repos owned by siafoundation (explored, hosted, cluster, benchmark) for the naming conventions of rhp imports and here are my findings.

  1. siafoundation/renterd has the most inconsistencies most specifically in go.sia.tech/coreutils/rhp/vX and go.sia.tech/core/rhp/vX
    • rhpX, crhpvX, rhp for go.sia.tech/coreutils/rhp/vX
    • rhpX for go.sia.tech/renterd/internal/rhp/vX
    • rhpX,rhpvX and crhpvX for go.sia.tech/core/rhp/vX
  2. siafoundation/explored has some inconsistencies
    • crhpvX for go.sia.tech/coreutils/rhp/vX
    • rhpvX for go.sia.tech/renterd/internal/rhp/vX
    • rhpX and rhpvX for go.sia.tech/core/rhp/vX
  3. siafoundation/hostd seems mostly consistent
    • rhpX for go.sia.tech/core/rhp/vX
    • rhpX for go.sia.tech/renterd/internal/rhp/vX
    • rhpX and protoX for go.sia.tech/core/rhp/vX
  4. siafoundation/cluster did not have much imports but it was mostly consistent
    • rhpX for go.sia.tech/core/rhp/vX
    • rhpX for go.sia.tech/core/rhp/vX
  5. siafoundation/benchmark seems mostly consistent
    • rhpX for go.sia.tech/core/rhp/vX
    • rhpX for go.sia.tech/renterd/internal/rhp/vX
    • rhpX and protoX for go.sia.tech/core/rhp/vX

Before I make a PR I would like your opinion on how I should approach the renaming process since there isn't really a clear convention throughout the various repositories I went through. The approach I came up with was to use curhpvX for go.sia.tech/coreutils/rhp/vX, crhpvX for go.sia.tech/coreutils/rhp/vX, and rhpvX for go.sia.tech/renterd/internal/rhp/vX to have a clear distinction if it's from coreutils, core, or internal and it will also eliminate any naming issues if there is a file with imports from more than one of these packages. If multiple and duplicate rhp version imports in the same file won't be an issue, then we can adopt a unified naming approach rhpX, since that's mainly used in the other repos. Here is a txt file that has raw data on my findings: rhp.txt

@ChrisSchinnerl
Copy link
Member Author

@ahuangg thanks for your help! This is super helpful already. It seems like you listed hostd twice but maybe you meant walletd the second time?

I think we can agree on proto being a good choice for core but the annoying conflict comes from internal and coreutils clashing. @n8maninger do you have any preference?

We could do

  • proto and protoutils (like io and ioutils) and rhp for internal
  • proto, rhp and irhp which is kinda nasty
  • something entirely different

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
Status: Backlog
Development

No branches or pull requests

2 participants