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

Repo.ID is int64, but several consumers are int #2804

Open
schmidtw opened this issue Jun 11, 2023 · 2 comments
Open

Repo.ID is int64, but several consumers are int #2804

schmidtw opened this issue Jun 11, 2023 · 2 comments

Comments

@schmidtw
Copy link

In the v53.0.0 release, the Repository.ID is defined as an int64, but there are several consumers are presently defined as int.

For most users this isn't a huge problem, but if you're running on a 32 bit system it will be an issue & also requires an explicit cast when one shouldn't be needed.

Affected methods:

func (s *ActionsService) CreateEnvVariable(ctx context.Context, repoID int, env string, variable *ActionsVariable) (*Response, error)
func (s *ActionsService) CreateOrUpdateEnvSecret(ctx context.Context, repoID int, env string, eSecret *EncryptedSecret) (*Response, error)

func (s *ActionsService) DeleteEnvSecret(ctx context.Context, repoID int, env, secretName string) (*Response, error)
func (s *ActionsService) DeleteEnvVariable(ctx context.Context, repoID int, env, variableName string) (*Response, error)

func (s *ActionsService) GetEnvPublicKey(ctx context.Context, repoID int, env string) (*PublicKey, *Response, error)
func (s *ActionsService) GetEnvSecret(ctx context.Context, repoID int, env, secretName string) (*Secret, *Response, error)
func (s *ActionsService) GetEnvVariable(ctx context.Context, repoID int, env, variableName string) (*ActionsVariable, *Response, error)

func (s *ActionsService) ListEnvSecrets(ctx context.Context, repoID int, env string, opts *ListOptions) (*Secrets, *Response, error)
func (s *ActionsService) ListEnvVariables(ctx context.Context, repoID int, env string, opts *ListOptions) (*ActionsVariables, *Response, error)

func (s *ActionsService) UpdateEnvVariable(ctx context.Context, repoID int, env string, variable *ActionsVariable) (*Response, error)
@gmlewis
Copy link
Collaborator

gmlewis commented Jun 11, 2023

I believe we need to standardize on 64 bits for all IDs.
I don't understand why you say this will be a problem on 32 bit systems.

The real problem is if GitHub ever uses a repo ID that doesn't fit in a 32 bit integer, and should not affect which architecture this code is running on, so I believe this is a very low priority issue. That is, unless you can show me otherwise.

@schmidtw
Copy link
Author

You're right that if the repo ID is over 32-bits there can be a problem. I'm not sure how large the IDs are.

An int is 64 bits on a 64-bit system, but is only 32 bits on a 32-bit system, and that's the reason the architecture causes a problem.

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

No branches or pull requests

2 participants