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

Move files from PR folder to the Forge folder #7378

Merged
merged 1 commit into from
Feb 25, 2025
Merged

Conversation

Caleb-T-Owens
Copy link
Contributor

There are several thoughts to be had here.

getPr.svelte.ts

This is a file that helps with accessing the forge services, giving a simpler interface.

  • Should this be a more generic catch all forgePreview.svelte.ts as it is bridging the gap between services and svelte?
  • Should this live in the base of forge/ or should there be a forge/pr/ folder which groups PR related things?

prContents.svelte.ts

This file is tied strongly to the PrDetailsModal.svelte.

  • Should this live in a folder with that PrDetailsModal.svelte file
  • Should this be more library like?

Conclusion...

There are lots of permutations, and I think for now, I'm content to just put them in the forge folder and visit these questions later as there are pros and cons with all of the options

Copy link

vercel bot commented Feb 24, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
gitbutler-components ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 25, 2025 3:01pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
gitbutler-web ⬜️ Skipped (Inspect) Feb 25, 2025 3:01pm

There are several thoughts to be had here.

## getPr.svelte.ts

This is a file that helps with accessing the forge services, giving a simpler interface.

- Should this be a more generic catch all `forgePreview.svelte.ts` as it is bridging the gap between services and svelte?
- Should this live in the base of `forge/` or should there be a `forge/pr/` folder which groups PR related things?

## prContents.svelte.ts

This file is tied strongly to the PrDetailsModal.svelte.

- Should this live in a folder with that `PrDetailsModal.svelte` file
- Should this be more library like?

## Conclusion...

![](https://media1.giphy.com/media/v1.Y2lkPTc5MGI3NjExeDJrNWFnd2k5djJncmN3aWx2Zjdqemdta2ZsNTdvbnNlZ2RzM3ZqbCZlcD12MV9pbnRlcm5hbF9naWZfYnlfaWQmY3Q9Zw/K6VhXtbgCXqQU/giphy.gif)

There are lots of permutations, and I think for now, I'm content to just put them in the forge folder and visit these questions later as there are pros and cons with all of the options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant