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

update the contributing & readme files, improve makefile #79

Merged
merged 9 commits into from
Apr 21, 2022

Conversation

fho
Copy link
Contributor

@fho fho commented Apr 21, 2022

make: remove printing '\n' instead of a newline

because -e was not passed to the "echo" call in "make list", it was
printing '\n' instead of a newline.

Remove printing "\n", printing an additional newline also does not make
the output more beautiful

-------------------------------------------------------------------------------
make: document the check target

The comment is used as documentation for the output of "make list".
When a target has no comment, it is not shown.

-------------------------------------------------------------------------------
readme: correct note about directory containing examples.

The directory is named "_examples" not "examples", also use a link as
reference

-------------------------------------------------------------------------------
readme: reference the CONTRIBUTING.md file

Instead of duplicating the information that is already in the
CONTRIBUTING.md file, reference the CONTRIBUTING.md file.
Also remove the sentence that TravisCI is used, it is not.

-------------------------------------------------------------------------------
docs: add a top-level header to CONTRIBUTING.md

-------------------------------------------------------------------------------
docs: update contributing docs regarding running static checks

- remove note about installing "golint", it is not needed
- add a chapter about running the static checks similar to the one about
  running integration tests,
- mention it in the workflow

-------------------------------------------------------------------------------
remove the pre-commit script

The script seems very outdated:
- it seems unused(?)
- it only runs some tools when the very old go version 1.11 is used, that the
  script considers as the stable version, go 1.11 is not even supported
  anymore, in go.mod the min. version is to 1.16
- it runs golint and govet which are also run by golangci-lint,
- it does not use the Makefile at all

Remove it also from the CONTRIBUTING.md file.

-------------------------------------------------------------------------------
docs: update note regarding integration tests in CONTRIBUTING.md

Clarify the documentation, make it clear that test-servers can be
reachable also via other URLs, document that "make tests" can be used.

-------------------------------------------------------------------------------
make: don't set AMQP_URL environment variable

Don't set the AMQP_URL environment variable when running "make tests".
It overwrote the environment variable and made it impossible to use
another URL + "make tests"
-------------------------------------------------------------------------------
integration tests: define default amqp url explicitly

1. If the AMQP_URL environment variable is not set, use
   amqp://guest:[email protected]:5672/ as default and print an informal
   message to stdout
2. Simplify it and don't duplicate retrieving the AMQP_URL from an
   environment variable. A amqpURL global variable is now defined that
   is set to the value of the AMQP_URL enviroment variable in an init()
   function. Tests function can reference the global variable instead of
   retrieving the env variable every time.

fho added 8 commits April 21, 2022 18:16
1. If the AMQP_URL environment variable is not set, use
   amqp://guest:[email protected]:5672/ as default and print an informal
   message to stdout
2. Simplify it and don't duplicate retrieving the AMQP_URL from an
   environment variable. A amqpURL global variable is now defined that
   is set to the value of the AMQP_URL enviroment variable in an init()
   function. Tests function can reference the global variable instead of
   retrieving the env variable every time.
Don't set the AMQP_URL environment variable when running "make tests".
It overwrote the environment variable and made it impossible to use
another URL + "make tests"
Clarify the documentation, make it clear that test-servers can be
reachable also via other URLs, document that "make tests" can be used.
The script seems very outdated:
- it seems unused(?)
- it only runs some tools when the very old go version 1.11 is used, that the
  script considers as the stable version, go 1.11 is not even supported
  anymore, in go.mod the min. version is to 1.16
- it runs golint and govet which are also run by golangci-lint,
- it does not use the Makefile at all

Remove it also from the CONTRIBUTING.md file.
- remove note about installing "golint", it is not needed
- add a chapter about running the static checks similar to the one about
  running integration tests,
- mention it in the workflow
Instead of duplicating the information that is already in the
CONTRIBUTING.md file, reference the CONTRIBUTING.md file.
Also remove the sentence that TravisCI is used, it is not.
The directory is named "_examples" not "examples", also use a link as
reference
CONTRIBUTING.md Outdated Show resolved Hide resolved
@fho fho marked this pull request as ready for review April 21, 2022 16:26
@fho fho changed the title docs: Update the contributing, readme, improve makefile docs: Update the contributing & readme files, improve makefile Apr 21, 2022
@fho fho changed the title docs: Update the contributing & readme files, improve makefile update the contributing & readme files, improve makefile Apr 21, 2022
@lukebakken
Copy link
Contributor

Thanks!

@lukebakken lukebakken merged commit f123594 into rabbitmq:main Apr 21, 2022
@lukebakken lukebakken added this to the 1.3.5 milestone Apr 21, 2022
@lukebakken lukebakken self-assigned this Apr 21, 2022
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

Successfully merging this pull request may close these issues.

2 participants