update style recommendations for bangs

[mlubin]

Some proposed updates based on the discussion in jump-dev/MathOptInterface.jl#475.

@IainNZ @matbesancon @juan-pablo-vielma @rdeits @IssamT

[rdeits]

I agree that this is an improvement 🙂

[codecov]

Codecov Report

Merging #1431 into master will increase coverage by 0.19%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1431      +/-   ##
==========================================
+ Coverage   88.88%   89.07%   +0.19%     
==========================================
  Files          25       25              
  Lines        3680     3928     +248     
==========================================
+ Hits         3271     3499     +228     
- Misses        409      429      +20

Impacted Files	Coverage Δ
src/macros.jl	89.64% <0%> (+0.01%)	⬆️
src/quadexpr.jl	92.03% <0%> (+0.46%)	⬆️
src/JuMP.jl	78.85% <0%> (+0.65%)	⬆️
src/parseexpr.jl	95.96% <0%> (+2.39%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f9bd39...f89c0de. Read the comment docs.

[blegat]

I agree with the change, ! is used to help readability, it is not an obligation and should not be used when redundant

[matbesancon]

Nice addition to the explanation. This was not in the initial discussion, but I would add putting the mutated argument first, like do_magic!(model, x, y). I found it a lot clearer in other packages to be able to identify it that way

[IainNZ]

lgtm

[mlubin]

! is used to help readability, it is not an obligation and should not be used when redundant

To clarify, we should try to reduce scope for personal preferences when applying the rules the style guide. There's still a question of if a name clearly implies modification, but we should we should either omit ! or use ! based on the answer to that question.

[mlubin]

@matbesancon I agree the ordering point is important. I believe that's addressed by the link to the corresponding discussion in the Julia style guide.

[matbesancon]

that's correct I had missed this section