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

Improved integration #5893

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Improved integration #5893

wants to merge 3 commits into from

Conversation

Caleb-T-Owens
Copy link
Contributor

@Caleb-T-Owens Caleb-T-Owens commented Jan 6, 2025


This is part 2 of 2 in a stack made with GitButler:

Copy link

vercel bot commented Jan 6, 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 Jan 9, 2025 11:24am
gitbutler-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 9, 2025 11:24am

@Caleb-T-Owens
Copy link
Contributor Author

Old time:
13.4s
New time:
107ms

I don't know how.

@Caleb-T-Owens Caleb-T-Owens requested review from Byron and krlvi and removed request for Byron January 8, 2025 13:20
@Caleb-T-Owens
Copy link
Contributor Author

This makes working on gitlab downright usable

@Byron
Copy link
Collaborator

Byron commented Jan 8, 2025

Old time: 13.4s New time: 107ms

I don't know how.

That's crazy awesome 🙌! It's probably doing much less work than before to do it, which is great if one can get away with it. Maybe previously, it did way too much work as well.

107ms on a repo of GitLab size is probably no more than a merge or two.

@Caleb-T-Owens
Copy link
Contributor Author

fuck... it was too good to be true.

this is indeed slower, and is slower by a fair bit.

@Caleb-T-Owens
Copy link
Contributor Author

@Byron / @krlvi Shall we still merge this despite the performance issues as it's behind a feature flag anyway.

@Byron
Copy link
Collaborator

Byron commented Jan 8, 2025

I think there is serious value in being able to detect if commits are integrated despite squashes. Of course, I am still concerned about the high runtime, which was high before and now is even higher.

@Caleb-T-Owens
Copy link
Contributor Author

Of course, I am still concerned about the high runtime, which was high before and now is even higher.

Indeed. I wouldn't consider removing the feature flag that it is currently under without some serious performance improvements.

Perhaps however it's just unwanted overhead with the fork in the logic. I just don't really want it to go stale

@Caleb-T-Owens
Copy link
Contributor Author

@krlvi wdyt?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@gitbutler/desktop rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants