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

Roadmap // Planned Styles & Credo replacements #18

Closed
14 of 17 tasks
novaugust opened this issue Apr 19, 2023 · 11 comments
Closed
14 of 17 tasks

Roadmap // Planned Styles & Credo replacements #18

novaugust opened this issue Apr 19, 2023 · 11 comments

Comments

@novaugust
Copy link
Contributor

novaugust commented Apr 19, 2023

As this project is in its infancy, we aren't actively looking for contributions in these areas, but feel free to reach out if you're interested in trying to implement a style

Def styles

  • Credo.Check.Readability.ParenthesesOnZeroArityDefs def foo() do -> def foo do (matt hates this rule but the community differs)
  • Credo.Check.Readability.PreferImplicitTry def fun do x rescue end w/ no try do
  • Credo.Check.Consistency.ParameterPatternMatching: def foo({:ok, _} = success), not def foo(success = {:ok, _})

Block styles

  • Credo.Check.Refactor.UnlessWithElse
  • Credo.Check.Refactor.NegatedConditionsInUnless
  • Credo.Check.Refactor.NegatedConditionsWithElse
  • Credo.Check.Refactor.CondStatementsrewrite 2 stanza cond dos as if statements if the second stanza is an atom:acond do x == y -> :ok; true -> false end
  • Credo.Check.Refactor.WithClauses first and last checks should be matches, move others up/down
  • Credo.Check.Refactor.RedundantWithClauseResultif the body of a with statement matches its last match, make the last match the body (important: can't do this if there are else clauses)

Invocation Styles styles

Some may better fit into Pipes, others Simple

  • Enum.into(%{}(, ...)) -> Map.new
  • Enum.reverse(x) ++ y -> Enum.reverse(x, y)
  • Credo.Check.... filter |> count => count(fn...end)
  • Credo.Check.Warning.ApplicationConfigInModuleAttribute
  • Credo.Check.Refactor.Apply WONTDO, using apply can help avoid circular dependencies in a scary codebase and so is unavoidable
  • Credo.Check.Readability.PipeIntoAnonymousFunctions: can rewrite to use then
  • Credo.Check.Refactor.MapJoin x |> Enum.map(..) |> Enum.join(...) -> Enum.map_join(x)
  • Credo.Check.Warning.ExpensiveEmptyEnumCheck (Enum.count|length == 0)
@Nezteb
Copy link

Nezteb commented May 4, 2023

Credo.Check.Readability.ParenthesesOnZeroArityDefs def foo() do -> def foo do (matt hates this rule but the community differs)

Personally I also dislike that rule because:

That said, as long as I have a tool that can automatically fix/standardize it, I don't really care. 😄

@novaugust
Copy link
Contributor Author

hah, i already tried going against the grain and my team rebelled ^>^ but now i can at least write it the way i like to write it and yeah, leave it for styler to make it suite the team's rules ;)

@mhanberg
Copy link
Contributor

Credo.Check.Readability.ParenthesesOnZeroArityDefs def foo() do -> def foo do (matt hates this rule but the community differs)

Personally I also dislike that rule because:

That said, as long as I have a tool that can automatically fix/standardize it, I don't really care. 😄

That rule refers to function definitions but the warning you mentioned is for local function calls.

  • It's less explicit.

I think you mean "verbose" 😉

@novaugust
Copy link
Contributor Author

novaugust commented May 11, 2023

mitch to think i thought you'd be coming here to be productive >.> :D

That rule refers to function definitions but the warning you mentioned is for local function calls.

yeah, that's the whole point for me though. > 0 arity functions are consistently defined and invoked, but 0 arity functions are defined differently from how they're invoked (per credo rule) ¯\_(ツ)_/¯. that inconsistency between defining 0 and > 0 funs has just failed to get into my muscle memory ("i just wrote a word after def, time to make an open parens! ... oops no wait no args so get rid of that")

but with styler, it doesn't matter! horay! (edit: uh, after i write that style, that is)

@mhanberg
Copy link
Contributor

I have the Midas touch.

Dev workflow is a valid argument to me for the rule existing.

I just don't think one way or the other is anymore explicit than the other.

@novaugust
Copy link
Contributor Author

Yeah, explicit doesn't enter into it for me. the def is pretty dang explicit :) I'm just sad i've never trained my fingers to do it in the way that credo likes

@adkron
Copy link

adkron commented Jun 15, 2023

The empty parens tear me. I don't particularly appreciate seeing them when there are no arguments, but as functions evolve in design, they tend to take an argument unless they are an impure function.

Many of my decisions on how I like to code involve incidental changes. Needing to add parens later is an incidental change. It has nothing to do with the work that I'm doing and doesn't change the communication abilities of the code. I have started leaning toward having parens for that very reason. It is one less thing to consider when making a change and one less piece of noise in a review. It seems like a tiny amount, but it makes a difference, and anything I can take away when looking at changes, the better.

@apoorv-2204
Copy link

when did adobe got into elixir? 😅

@Nezteb
Copy link

Nezteb commented Jul 12, 2023

when did adobe got into elixir? 😅

@apoorv-2204 Since they acquired https://frame.io maybe? 😄

@apoorv-2204
Copy link

when did adobe got into elixir? sweat_smile

@apoorv-2204 Since they acquired https://frame.io maybe? 😄

😅

Is/Will Adobe adopting elixir for their mainstream products? 😅

@novaugust novaugust mentioned this issue Aug 2, 2023
4 tasks
@novaugust
Copy link
Contributor Author

At this point I've implemented all but two (very minor ones) of the things in here, so I'm closing the tracker :)

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

No branches or pull requests

5 participants