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

Review prompter internals #2630

Merged
merged 20 commits into from
Nov 18, 2022
Merged

Review prompter internals #2630

merged 20 commits into from
Nov 18, 2022

Conversation

aadcg
Copy link
Member

@aadcg aadcg commented Nov 9, 2022

Description

This is a followup to #2555, since I had to review and reason about most aspects related to the prompter.

Checklist:

Everything in this checklist is required for each PR. Please do not approve a PR that does not have all of these items.

  • I have pulled from master before submitting this PR
  • There are no merge conflicts.
  • I've added the new dependencies as:
    • ASDF dependencies,
    • Git submodules,
      cd /path/to/nyxt/checkout
      git submodule add https://gitlab.common-lisp.net/nyxt/py-configparser _build/py-configparser
    • and Guix dependencies.
  • My code follows the style guidelines for Common Lisp code. See:
  • I have performed a self-review of my own code.
  • My code has been reviewed by at least one peer. (The peer review to approve a PR counts. The reviewer must download and test the code.)
  • Documentation:
    • All my code has docstrings and :documentations written in the aforementioned style. (It's OK to skip the docstring for really trivial parts.)
    • I have updated the existing documentation to match my changes.
    • I have commented my code in hard-to-understand areas.
    • I have updated the changelog.lisp with my changes if it's anything user-facing (new features, important bug fix, compatibility breakage).
    • I have added a migration.lisp entry for all compatibility-breaking changes.
  • Compilation and tests:
    • My changes generate no new warnings.
    • I have added tests that prove my fix is effective or that my feature works. (If possible.)
    • New and existing unit tests pass locally with my changes.

@aadcg
Copy link
Member Author

aadcg commented Nov 9, 2022

It's not clear to me whether the fact that return-actions is ensured to be a list needs to be added to the changelog, since it belongs to the prompter library which is external to Nyxt.

@aadcg aadcg mentioned this pull request Nov 9, 2022
.gitignore Outdated Show resolved Hide resolved
libraries/prompter/prompter-source.lisp Outdated Show resolved Hide resolved
libraries/prompter/prompter-source.lisp Show resolved Hide resolved
libraries/prompter/prompter.lisp Outdated Show resolved Hide resolved
libraries/prompter/prompter.lisp Show resolved Hide resolved
source/buffer.lisp Show resolved Hide resolved
source/buffer.lisp Show resolved Hide resolved
source/describe.lisp Show resolved Hide resolved
source/mode/autofill.lisp Outdated Show resolved Hide resolved
source/mode/document.lisp Show resolved Hide resolved
@Ambrevar
Copy link
Member

Nothing to add :)

@aadcg aadcg force-pushed the review-prompter-internals branch from 1e6e601 to 51e5933 Compare November 18, 2022 13:33
@aadcg aadcg merged commit 7912fa0 into master Nov 18, 2022
@aadcg aadcg deleted the review-prompter-internals branch November 18, 2022 13:34
@aadcg
Copy link
Member Author

aadcg commented Nov 18, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants