-
Notifications
You must be signed in to change notification settings - Fork 43
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
chore: address golangci-lint issues #74
Conversation
@@ -96,13 +96,6 @@ func (b Bubble) Update(msg tea.Msg) (Bubble, tea.Cmd) { | |||
cmds []tea.Cmd | |||
) | |||
|
|||
switch msg := msg.(type) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This block intrigued me. I left it untouched knowing I would add golangci-lint, do it would be reported
@@ -66,6 +66,7 @@ func (b Bubble) View() string { | |||
return b.Styles.containerStyle.Render(b.textinput.View()) | |||
} | |||
|
|||
//nolint:revive //will refactor later |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A TODO maybe
Also, you should write what was the issue, I mean the revive rules
I think its about code complexity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah i definitely just punted on it for now while I come up with something more thoughtful.
@@ -31,64 +31,84 @@ type copyQueryToClipboardMsg struct{} | |||
|
|||
type copyResultsToClipboardMsg struct{} | |||
|
|||
// executeQuery executes a query using the provided query input and input data, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's great that you started refactoring because of what golangci-lint reported. I assumed it was something about complexity.
I love linting because it allows to realize you can write better code.
@@ -149,7 +149,7 @@ linters-settings: | |||
|
|||
# https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#cyclomatic | |||
- name: cyclomatic | |||
arguments: [5] # default 3 | |||
arguments: [8] # default 3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you could go back to 3 or 5, by simply adding //nolint:revive // this block is to complex for now. TODO refactorit later
where needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I thought about staying at 5
or reverting to 3
but I was thinking about it and couldn't really come up with what the ideal number should be. For example, 3
feels low and for example I find a switch statement in the code with with 5 clauses with not lots of nested logic fairly readable but it would fail this linting check. I guess I could refactor it more to use like a map with keys that would be checked in the switch and values of functions to run but not sure that is any better in terms of readability/maintainability. I think in the very large switches with nested logic, that approach makes sense fwiw, but not on something I guess I would categorize as already readable/maintainable. wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a matter of balance. You are the only one who can figure the right value here.
Having 8 sounds an high value to me, I prefer having code reported, but excluding with a nolint because they cannot be simplified than having a value too high.
Think about people who may contribute and come with spaghetti code that will go under the 8 value
|
||
highlightedOutput, err := utils.Prettify([]byte(results.String()), b.theme.ChromaStyle, true) | ||
if !b.isJSONLines { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest rewriting it like that
var processor context.Context, *strings.Builder, *gojq.Query, []byte) error = processJSONWithQuery
if b.isJSONLines {
processor = processJSONLinesWithQuery
}
if err := processor(ctx, &results, query, b.inputdata.GetInputJSON()); err != nil {
return "", err
}
return results.String(), nil
(pseudo code written on a phone)
@@ -34,6 +34,7 @@ func (b *Bubble) resizeBubbles() { | |||
b.output.SetSize(b.width/2, height) | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same remark about a possible TODO to add here
👍 |
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [noahgorstein/jqp](https://github.com/noahgorstein/jqp) | minor | `v0.6.0` -> `v0.7.0` | --- ### Release Notes <details> <summary>noahgorstein/jqp (noahgorstein/jqp)</summary> ### [`v0.7.0`](https://github.com/noahgorstein/jqp/releases/tag/v0.7.0) [Compare Source](https://github.com/noahgorstein/jqp/compare/v0.6.0...v0.7.0) #### Overview Mostly small bug fixes and some performance improvements. One new feature added is the ability to specify an optional query argument to `jqp` cli that it will execute on startup. curl "https://api.github.com/repos/jqlang/jq/issues" | jqp '.[] | {"title": .title, "url": .url}' https://github.com/noahgorstein/jqp/assets/23270779/735cf84a-dc6f-41e8-9659-5022a4937046 [@​EmilyGraceSeville7cf](https://github.com/EmilyGraceSeville7cf) also added a JSON schema for jqp's config which should help users create and edit their config files. #### What's Changed - typos suggestion by [@​ccoVeille](https://github.com/ccoVeille) in [https://github.com/noahgorstein/jqp/pull/59](https://github.com/noahgorstein/jqp/pull/59) - code review by [@​ccoVeille](https://github.com/ccoVeille) in [https://github.com/noahgorstein/jqp/pull/61](https://github.com/noahgorstein/jqp/pull/61) - Add Continuous Integration to GitHub actions by [@​ccoVeille](https://github.com/ccoVeille) in [https://github.com/noahgorstein/jqp/pull/63](https://github.com/noahgorstein/jqp/pull/63) - Add GitHub actions for checking typos by [@​ccoVeille](https://github.com/ccoVeille) in [https://github.com/noahgorstein/jqp/pull/62](https://github.com/noahgorstein/jqp/pull/62) - Add dependabot to GitHub Action by [@​ccoVeille](https://github.com/ccoVeille) in [https://github.com/noahgorstein/jqp/pull/64](https://github.com/noahgorstein/jqp/pull/64) - chore: address golangci-lint issues by [@​noahgorstein](https://github.com/noahgorstein) in [https://github.com/noahgorstein/jqp/pull/74](https://github.com/noahgorstein/jqp/pull/74) - chore(deps): bump github.com/itchyny/gojq from 0.12.13 to 0.12.15 by [@​dependabot](https://github.com/dependabot) in [https://github.com/noahgorstein/jqp/pull/72](https://github.com/noahgorstein/jqp/pull/72) - chore(deps): bump github.com/alecthomas/chroma/v2 from 2.12.0 to 2.13.0 by [@​dependabot](https://github.com/dependabot) in [https://github.com/noahgorstein/jqp/pull/73](https://github.com/noahgorstein/jqp/pull/73) - chore(deps): bump github.com/charmbracelet/bubbles from 0.16.1 to 0.18.0 by [@​dependabot](https://github.com/dependabot) in [https://github.com/noahgorstein/jqp/pull/71](https://github.com/noahgorstein/jqp/pull/71) - chore(deps): bump github.com/charmbracelet/lipgloss from 0.8.0 to 0.10.0 by [@​dependabot](https://github.com/dependabot) in [https://github.com/noahgorstein/jqp/pull/70](https://github.com/noahgorstein/jqp/pull/70) - chore(deps): bump github.com/spf13/cobra from 1.5.0 to 1.8.0 by [@​dependabot](https://github.com/dependabot) in [https://github.com/noahgorstein/jqp/pull/69](https://github.com/noahgorstein/jqp/pull/69) - chore(deps): bump github.com/spf13/viper from 1.13.0 to 1.18.2 by [@​dependabot](https://github.com/dependabot) in [https://github.com/noahgorstein/jqp/pull/68](https://github.com/noahgorstein/jqp/pull/68) - chore(deps): bump github.com/charmbracelet/bubbletea from 0.24.1 to 0.26.2 by [@​dependabot](https://github.com/dependabot) in [https://github.com/noahgorstein/jqp/pull/67](https://github.com/noahgorstein/jqp/pull/67) - chore(deps): bump actions/setup-go from 4 to 5 by [@​dependabot](https://github.com/dependabot) in [https://github.com/noahgorstein/jqp/pull/66](https://github.com/noahgorstein/jqp/pull/66) - chore: upgrade to go v1.22 by [@​noahgorstein](https://github.com/noahgorstein) in [https://github.com/noahgorstein/jqp/pull/75](https://github.com/noahgorstein/jqp/pull/75) - refactor: reduce complexity of various methods/functions by [@​noahgorstein](https://github.com/noahgorstein) in [https://github.com/noahgorstein/jqp/pull/76](https://github.com/noahgorstein/jqp/pull/76) - feat: add optional cli argument to specify initial query by [@​noahgorstein](https://github.com/noahgorstein) in [https://github.com/noahgorstein/jqp/pull/77](https://github.com/noahgorstein/jqp/pull/77) - bug: dont set viewport content on resize by [@​noahgorstein](https://github.com/noahgorstein) in [https://github.com/noahgorstein/jqp/pull/79](https://github.com/noahgorstein/jqp/pull/79) - chore(deps): bump github.com/itchyny/gojq from 0.12.15 to 0.12.16 by [@​dependabot](https://github.com/dependabot) in [https://github.com/noahgorstein/jqp/pull/85](https://github.com/noahgorstein/jqp/pull/85) - chore(deps): bump github.com/alecthomas/chroma/v2 from 2.13.0 to 2.14.0 by [@​dependabot](https://github.com/dependabot) in [https://github.com/noahgorstein/jqp/pull/80](https://github.com/noahgorstein/jqp/pull/80) - chore(deps): bump github.com/charmbracelet/lipgloss from 0.10.0 to 0.11.0 by [@​dependabot](https://github.com/dependabot) in [https://github.com/noahgorstein/jqp/pull/82](https://github.com/noahgorstein/jqp/pull/82) - chore(deps): bump github.com/spf13/viper from 1.18.2 to 1.19.0 by [@​dependabot](https://github.com/dependabot) in [https://github.com/noahgorstein/jqp/pull/84](https://github.com/noahgorstein/jqp/pull/84) - chore(deps): bump github.com/charmbracelet/bubbletea from 0.26.2 to 0.26.4 by [@​dependabot](https://github.com/dependabot) in [https://github.com/noahgorstein/jqp/pull/83](https://github.com/noahgorstein/jqp/pull/83) - fix: revert to lipgloss v0.10.0 temporarily by [@​noahgorstein](https://github.com/noahgorstein) in [https://github.com/noahgorstein/jqp/pull/87](https://github.com/noahgorstein/jqp/pull/87) - fix: don't block while inputdata bubble initializes by [@​noahgorstein](https://github.com/noahgorstein) in [https://github.com/noahgorstein/jqp/pull/86](https://github.com/noahgorstein/jqp/pull/86) - fix: nil pointer dereference as a result of accessing state before queryinput initialized by [@​noahgorstein](https://github.com/noahgorstein) in [https://github.com/noahgorstein/jqp/pull/90](https://github.com/noahgorstein/jqp/pull/90) - fix: dynamically increase buffer size to handle processing large JSON lines by [@​noahgorstein](https://github.com/noahgorstein) in [https://github.com/noahgorstein/jqp/pull/93](https://github.com/noahgorstein/jqp/pull/93) - chore(deps): bump github.com/charmbracelet/lipgloss from 0.10.0 to 0.11.0 by [@​dependabot](https://github.com/dependabot) in [https://github.com/noahgorstein/jqp/pull/89](https://github.com/noahgorstein/jqp/pull/89) - chore(deps): bump crate-ci/typos from 1.21.0 to 1.22.1 by [@​dependabot](https://github.com/dependabot) in [https://github.com/noahgorstein/jqp/pull/92](https://github.com/noahgorstein/jqp/pull/92) - feat: implement json schema for config by [@​EmilyGraceSeville7cf](https://github.com/EmilyGraceSeville7cf) in [https://github.com/noahgorstein/jqp/pull/78](https://github.com/noahgorstein/jqp/pull/78) - chore: prep v0.7.0 release by [@​noahgorstein](https://github.com/noahgorstein) in [https://github.com/noahgorstein/jqp/pull/94](https://github.com/noahgorstein/jqp/pull/94) #### New Contributors - [@​ccoVeille](https://github.com/ccoVeille) made their first contribution in [https://github.com/noahgorstein/jqp/pull/59](https://github.com/noahgorstein/jqp/pull/59) - [@​dependabot](https://github.com/dependabot) made their first contribution in [https://github.com/noahgorstein/jqp/pull/72](https://github.com/noahgorstein/jqp/pull/72) - [@​EmilyGraceSeville7cf](https://github.com/EmilyGraceSeville7cf) made their first contribution in [https://github.com/noahgorstein/jqp/pull/78](https://github.com/noahgorstein/jqp/pull/78) **Full Changelog**: noahgorstein/jqp@v0.6.0...v0.7.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/d-issy/dotfiles). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zOTMuMCIsInVwZGF0ZWRJblZlciI6IjM3LjM5My4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
No description provided.