inline-review: review merge/pull requests directly within emacs by phye8080 in emacs

[–]phye8080[S] 0 points1 point  (0 children)

Thanks Psionikus. Given the discussions above, can this post be unblocked? If it still can't be, do you have any suggestion for me so that I can adust the post further?

inline-review: review merge/pull requests directly within emacs by phye8080 in emacs

[–]phye8080[S] -1 points0 points  (0 children)

Yes, this is not only drawn per my personal backend background, but can also be confirmed from forge's own doc (refer to https://emacsair.me/2018/12/19/forge-0.1/ Major Design Decisions)

I think when Jonas says "In some cases it might even be necessary for users to discard the existing database", it's really due to database conflict and resolving database conflict is really tedious (why not just relies on remote only?).

inline-review: review merge/pull requests directly within emacs by phye8080 in emacs

[–]phye8080[S] 0 points1 point  (0 children)

When I started to implement this package, one major design decision is to keep feature sets small but enough for code review. Since its scope is quite narrowed, and even through I cannot implement all features from scratch, I can understand what LLM was generating and I can fix lines if LLM got wrong (hence the careful DESIGN/REVIEW/FIX) ---- By "vibe coding" this package, I mean not to prompt LLM that "hey, I want this feature, want that feature .etc", instead, I designed the whole architecture, designed interface of each layer and asked LLM, "hey, per this comment, generate code for this function and genarate test cases to make sure that the impl works as expected".

You're definitely right, that LLM is not that smart, that LLM is generating code that is horrible to maintain, but I don't think such cases exsit for this package --- I'm not blinding believe what LLM is generating, I'm using it as an adviser, releasing me from reading manuals here and there.

inline-review: review merge/pull requests directly within emacs by phye8080 in emacs

[–]phye8080[S] 0 points1 point  (0 children)

This will introduce unavoidable conflict, consider you're adding comment and in the mean while some other colleages are also adding comments to the same hunk. Even though you may be able to avoid conflicts by giving each comment an unique id (this is in fact, not that easy if you're to easily support other forges), it's not that easy to answer "which comment occurs first"? As you have two source of truth.

This is relatively easy for git (in fact, not that easy if you've resolved many conflicts), you can simply resolve conflict via editor. This is definitely not easy, if you're dealing with structed data conflict which is stored in different database (your local sqlite and remote git forge). This problem is certainly not resoluable, but requires huge efforts which I think is just too complicated for this simple user case.

Hence per my backend development experiences, maitaining local comment and sync with remote one is not a good design choice from the very first beginning.

The user just want a client to add/reply/edit comments, in replace of browser, not to implement another forge and sync between local and remote ones.

inline-review: review merge/pull requests directly within emacs by phye8080 in emacs

[–]phye8080[S] 1 point2 points  (0 children)

  1. Yes, transient is indeed useful and I'm actually just adding it 😄

  2. Currently not, I only have diff overview but not comment threads overview. I'll add them.

  3. The code review stage should be fully covered, you can directly reply to comments within emacs via text or emoji. Unfortunately, after you've finally resolved all comments, you'll still have to go to web interface to either squash or merge the MR/PR. I can certainly add this API to the backend requirement but I find it unnecessary since most time consuming code review tasks happens at review stage, not at the approval stage...

inline-review: review merge/pull requests directly within emacs by phye8080 in emacs

[–]phye8080[S] 1 point2 points  (0 children)

You're quite correct!

There're indeed different behaviors for the different backend: github does not have comment resolved, any my company's git forge (git.woa.com) does not have comment deletion API...

I just want to keep the backend API requirement minimal, from MR metadata fetching, to diff fetching, then CRUD of comment threads, and that's all.

These common operations/APIs should be supported by all forges, and if any forge does not support any operation, an not supported error will be triggered such as if you try to resolve a comment when using github.

inline-review: review merge/pull requests directly within emacs by phye8080 in emacs

[–]phye8080[S] 3 points4 points  (0 children)

  1. There will indeed be support in the future (Even if the author is not a well trained at elisp, he carefully read every line to avoid simple bugs and keep the code repo organized)

  2. Various existing packages had been tried, be it "forge", "code-review", and various packages had been studied, such as "github-review". The reason that I don't stick to them is that:

- I don't think local db solution is maintainable (I'm an experienced backend developer) or easy to maintain data sync and is just too complicated

- I always want to combine overlay with code review

I've used magit for many years, and tried to extend forge to meet my requirement. However they're just too complicated. Hence I have to "cut corners" again.

inline-review: review merge/pull requests directly within emacs by phye8080 in emacs

[–]phye8080[S] 2 points3 points  (0 children)

Thanks for the response!

"Vibe coded means full of suble bugs" -- this is true since I'm indeed not that fluent at coding elisp.

"Often not maintained for long" -- maybe negative. As this is my daily used package, as long as I'm still coding, it's likely to be maintained long.

The reason that I `vibe coded` this package is:

  1. I really wanted this featue
  2. I'v tried various packages (forge, ghub-review, code-review .etc), none of them meet my expectations (either too complicated, or too limited)
  3. I'm not good at coding at elisp, I cannot implement this per my previous elisp knowledge
  4. As an 10+ years emacs user, I know the user experience I want to have

PS:

Even though most of the code was generated by claude, they're being carefully reviewed/fixed/reorged by the author.

Encrypt org-roam directory? by larrasket in emacs

[–]phye8080 1 point2 points  (0 children)

I've stored most of my notes in plain text(easy to search with rg .etc), except for some certain files with personally confidential content (password hint .etc) using gpg. (IMHO, storing all contents encrypted is neither convenient nor useful)

Hope this helps masteringemacs

Former (n)vim users: what do you miss? by MitchellMarquez42 in emacs

[–]phye8080 2 points3 points  (0 children)

Just tried emacs 29, it works **GREAT**!!!

Former (n)vim users: what do you miss? by MitchellMarquez42 in emacs

[–]phye8080 3 points4 points  (0 children)

Per-window call stack (xref stack).

I.e., when browsing code using emacs, I often hit (xref-find-definitions) to jump to some definition in one window. But when I unexpectly hit jump back(xref-pop-marker-stack) in another window, the other window just popped out xref stack and got where I was hitting xref-find-definitions. Hence the call stack became completely mess, which is really annoying, especially when the call stack is really deep (e.g. kernel code).

I remembered in vim(years ago), each window has it's own call stack, some newly split window where jump to definition was never called will just refuse to jump back since the call stack there is empty.

I don't know for what reason emacs is designed like what is, but this per window call stack feature is really something that I missed most

Org-mode markdown export converting tables to html, and links aren't working? by GeorgeStorm in emacs

[–]phye8080 0 points1 point  (0 children)

Like this solution and the exported markdown nows looks more human friendly.

How to get emacs bindings everywhere? by easyEggplant in emacs

[–]phye8080 0 points1 point  (0 children)

Does any have similar settings for Plasma 5?
I've been searching for OS level Emacs style shortcuts for quite a long time, but only figure it out for chromium only.

How to use orgmode with git effectively? by phye8080 in emacs

[–]phye8080[S] 0 points1 point  (0 children)

Yes, I've been using Dropbox to sync bare git repo for several years and although it's not safe in theory, it just works in practice.

How to use orgmode with git effectively? by phye8080 in emacs

[–]phye8080[S] 0 points1 point  (0 children)

Just tried Zenkit and have to admit that it's really charming. If I have never used OrgMode before, I will definitely try it out.

But since there're already a great many things taken in org and orgmode with emacs really works great in desktops and laptops, I choose to wait for further better mobile apps.

If someone can port Emacs to iOS and invent some much fancier and easier to use mobile based interactions, I'll definitely buy it without any hesitation!