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

Merge shunf4/master #94

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

Merge shunf4/master #94

wants to merge 20 commits into from

Conversation

phil294
Copy link
Owner

@phil294 phil294 commented Apr 18, 2024

Hi @shunf4, am I estimating correctly that you've been modifying the extension for personal usage and do not plan on actively contributing to the master branch...? Because there are some interesting things you've added and fixed, but I also see UX/code problems and some changes that I disagree with that would make it impossible to just merge it.

So unless you want to convert all your changes into separate PRs to have each of them undergo some reviewing (or if I don't hear back from you), I'll just allow myself to make one big pseudo-PR, delete from it everything I don't want, fix some code sections myself as I see fit and move on.

Note that I would like to remove SLM and CoffeeScript and move to HTML and JS, respectively, (#62) so that will make merging stuff more difficult in the future. But I'll write some scripts / use existing tooling for that, so probably no manual action would be necessary.

Thanks for making your fork public! 👍

@shunf4
Copy link

shunf4 commented Apr 20, 2024

I made minimal changes to the code in my fork, only to bring certain features and fixes. Most of them are quite personal, but I think some of them can be taken by the upstream, like the fix for displaying moved files (path/{before/move/file.txt => after/move/file.txt}) in commit change list, using full hash to uniquely identify a commit, etc. So yes, please feel free to pick modification that look good in this PR and (if I could recall) ask me about anywhere you are unsure about its meaning.

Now that I've made most of the changes I need, I don't plan to put much efforts into my fork then. Probably not considering rebasing your big update in the future, only some small fixes and adjustments according to my need maybe.

Thank you for this great project.

@shunf4
Copy link

shunf4 commented Apr 20, 2024

d612631
7d77f4d

I think the graphing algorithm has defects and I failed to completely fix them. When the history graph gets complicated, some lines are missing or not connected to the dots.

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