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

ERL-151: Make erl_tidy work on input file with tilda (~) character. #1071

Closed
wants to merge 5 commits into
base: maint
from

Conversation

Projects
None yet
5 participants
@mbucc
Contributor

mbucc commented May 27, 2016

@uabboli uabboli self-assigned this May 27, 2016

@OTP-Maintainer

This comment has been minimized.

OTP-Maintainer commented May 27, 2016

The summary line of the commit message is too long and/or ends with a "."
Make sure the whole message follows the guidelines here: https://github.com/erlang/otp/wiki/Writing-good-commit-messages.

Bad message: ERL-151: Make erl_tidy work on input file with tilda (~) character.


I am a script, I am not human


Correct fix
The ~p format causes the output to include "\n" string instead of
the '\n' character.  Using put_chars/1 properly prints these
characters and works when input has the tilda character.
@OTP-Maintainer

This comment has been minimized.

OTP-Maintainer commented May 30, 2016

The summary line of the commit message is too long and/or ends with a "."
Make sure the whole message follows the guidelines here: https://github.com/erlang/otp/wiki/Writing-good-commit-messages.

Bad message: ERL-151: Make erl_tidy work on input file with tilda (~) character.


I am a script, I am not human


@proxyles

This comment has been minimized.

Contributor

proxyles commented May 31, 2016

this commit msg "ERL-151: Make erl_tidy work on input file with tilda (~) character." ends in a "."

@mbucc

This comment has been minimized.

Contributor

mbucc commented Jun 1, 2016

Just tried to rebase then ammend. FAIL. Is the terminating period blocking acceptance?

@dgud

This comment has been minimized.

Contributor

dgud commented Jun 2, 2016

Could you add a testcase, for instance the one in the ERL-151.
No the period is not blocking.

@mbucc

This comment has been minimized.

Contributor

mbucc commented Jun 4, 2016

Sure.

Is otp/lib/syntax_tools/test/syntax_tools_SUITE.erl where I should add the new test?

@OTP-Maintainer

This comment has been minimized.

OTP-Maintainer commented Jun 8, 2016

The summary line of the commit message is too long and/or ends with a "."
Make sure the whole message follows the guidelines here: https://github.com/erlang/otp/wiki/Writing-good-commit-messages.

Bad message: Remove array brackets that got in by mistake.
Add unit test that duplicates bug.
ERL-151: Make erl_tidy work on input file with tilda (~) character.


I am a script, I am not human


@mbucc

This comment has been minimized.

Contributor

mbucc commented Jun 15, 2016

@dgud: test committed.

@dgud

This comment has been minimized.

Contributor

dgud commented Jun 16, 2016

I saw that, we didn't have time to look at this before code freeze.

@uabboli

This comment has been minimized.

Contributor

uabboli commented Jul 4, 2016

A few things should be fixed before the patch is merged:

  • Replace 'tilda' with 'tilde'.
  • Squash all commits together.
  • Let the commit message begin with "syntax_tools: ".
    You can mention ERL-151 in the commit message
    ("ERL-151" will also be included in the final branch name).
  • Even if it's not strictly necessary, the test module should
    compile.
Fix spelling (tilde, not tilda)
Also, make input to erl_tidy test compile.
@OTP-Maintainer

This comment has been minimized.

OTP-Maintainer commented Jul 6, 2016

The summary line of the commit message is too long and/or ends with a "."
Make sure the whole message follows the guidelines here: https://github.com/erlang/otp/wiki/Writing-good-commit-messages.

Bad message: Remove array brackets that got in by mistake.
Add unit test that duplicates bug.
ERL-151: Make erl_tidy work on input file with tilda (~) character.


I am a script, I am not human


@uabboli

This comment has been minimized.

Contributor

uabboli commented Jul 7, 2016

Merged commit 0969eca into erlang:maint.

@uabboli uabboli closed this Jul 7, 2016

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