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

Draft: fix(alias): removed pointer for array alias types #1032

Closed
wants to merge 1 commit into from

Conversation

alesbrelih
Copy link
Contributor

Pointers were used on aliased arrays. This made casting difficult because after the cast handler expected pointer to the cast type.

Because slices are still passed as references and there is almost no allocating happening under the hood I thought it would be OK to change them to nonpointer.

Example before:

	apiPets := conv.Map(pets, func(pet pet.Pet) api.Pet {
		return api.Pet{
			ID:   pet.ID,
			Name: pet.Name,
			Tag:  api.NewOptString(pet.Tag),
		}
	})

	return conv.ToPointer(api.FindPetsOKApplicationJSON(apiPets)), nil

Example after:

	apiPets := conv.Map(pets, func(pet pet.Pet) api.Pet {
		return api.Pet{
			ID:   pet.ID,
			Name: pet.Name,
			Tag:  api.NewOptString(pet.Tag),
		}
	})

	return api.FindPetsOKApplicationJSON(apiPets), nil

If you prefer the pointer I could create a new helper func to be generated along with alias?

Your call.

Thanks!

@codecov
Copy link

codecov bot commented Sep 8, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.02% ⚠️

Comparison is base (ca166cb) 72.23% compared to head (d10a839) 72.21%.

❗ Current head d10a839 differs from pull request most recent head 2a81674. Consider uploading reports for the commit 2a81674 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1032      +/-   ##
==========================================
- Coverage   72.23%   72.21%   -0.02%     
==========================================
  Files         192      192              
  Lines       15003    15003              
==========================================
- Hits        10837    10835       -2     
- Misses       3631     3632       +1     
- Partials      535      536       +1     

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Pointers were used on aliased arrays. This made casting difficult because
after the cast handler expected pointer to the cast type.
@alesbrelih alesbrelih changed the title fix(alias): removed pointer for array alias types Draft: fix(alias): removed pointer for array alias types Sep 8, 2023
@alesbrelih
Copy link
Contributor Author

Closing this, found out I'm overthinking this.

@alesbrelih alesbrelih closed this Sep 8, 2023
@alesbrelih alesbrelih deleted the fix/alias-pointer branch September 8, 2023 22:16
@ernado
Copy link
Member

ernado commented Sep 9, 2023

Actually this would be pretty helpfull, pointers for aliased types are annoying.

@alesbrelih
Copy link
Contributor Author

Ok, I'll reopen this MR after I fix something.

Wasn't sure if you even want this change and I didn't want to "force" personal opinions.

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