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

tool: nimsuggest remove terse flag #898

Merged
merged 1 commit into from
Sep 17, 2023

Conversation

bung87
Copy link
Contributor

@bung87 bung87 commented Sep 13, 2023

Summary

Remove terse flag from nimsuggest. It's not used by any of the
existing extensions, nor is it tested.

Details

The option itself is rather arbitrary in what it filters and the design
is confusing in terms of determining what should and shouldn't qualify.

@saem saem changed the title remove nimsuggest flag terse remove terse flag from nimsuggest Sep 14, 2023
@saem saem added tool Improvements to non-compiler tooling simplification Removal of the old, unused, unnecessary or un/under-specified language features. labels Sep 14, 2023
@saem saem added this to the Tooling milestone Sep 14, 2023
@saem
Copy link
Collaborator

saem commented Sep 14, 2023

@bung87 please see the PR, there is a part of the original body that I can't work out what you mean, can you add more info, I'll help with the grammar?

Also, note how I changed the body, including reviewer notes. These messages are not simply for the PR review now, but also for future retrospectives, someone putting together change logs, learning from the compiler etc... so we ensure they do a good job describing the goal, change, and impact.

@saem
Copy link
Collaborator

saem commented Sep 14, 2023

After the PR body is updated this is good to merge, terse is pointless, so happy to see it go.

@bung87 bung87 changed the title remove terse flag from nimsuggest tool: nimsuggest remove terse flag Sep 16, 2023
@saem
Copy link
Collaborator

saem commented Sep 16, 2023

/merge

@github-actions
Copy link

Merge requested by: @saem

Contents after the first section break of the PR description has been removed and preserved below:


Notes for Reviewers

@chore-runner chore-runner bot added this pull request to the merge queue Sep 16, 2023
Merged via the queue into nim-works:devel with commit f032b96 Sep 17, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
simplification Removal of the old, unused, unnecessary or un/under-specified language features. tool Improvements to non-compiler tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants