Skip to content

Conversation

@sicoyle
Copy link
Contributor

@sicoyle sicoyle commented Sep 23, 2022

What does this PR do?
This PR is meant to improve consistency in documentation and make minor typo adjustments.

Why is it important?
Enhance dev understanding and readability in documentation.

Related issues
None

@sicoyle sicoyle requested a review from a team as a code owner September 23, 2022 22:10
Copy link
Member

@kiview kiview left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, very much appreciated.

@mdelapenya mdelapenya added the documentation Docs, docs, docs. label Sep 24, 2022
Copy link
Member

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for improving the docs, it's super important for the project

LGTM

@codecov
Copy link

codecov bot commented Sep 24, 2022

Codecov Report

Merging #534 (b5570b9) into main (1486bcc) will increase coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #534      +/-   ##
==========================================
+ Coverage   68.71%   68.76%   +0.04%     
==========================================
  Files          22       22              
  Lines        2148     2148              
==========================================
+ Hits         1476     1477       +1     
  Misses        532      532              
+ Partials      140      139       -1     
Impacted Files Coverage Δ
testing.go 0.00% <ø> (ø)
docker.go 70.82% <0.00%> (+0.10%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

# Build from Dockerfile

Testcontainers-go gives you the ability to build an image and run a container
Testcontainers-Go gives you the ability to build an image and run a container
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kiview I think we should standardize the naming to one of Testcontainers-Go or Testcontainers-go.

As we are not using Testcontainers-Java, I'd use the lowercase version for the language.

WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering the same when I saw the PR. In Java, if we talk about the library in a more technical context, we tend to call it testcontainers-java, so all lowercase.

But the general name of the project is Testcontainers, so upper cased.

I think I am with you, when talking about Testcontainers-go in a more literate way, lower casing the language looks better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can lower case it then! I am glad this discussion is being had because I have created presentations to sister teams at work on this pkg, and have often wondered which were the proper route to take.

Also, fwiw, I'll be presenting at GopherCon (US) on my team's integration testing solution which incorporates this pkg! So if there are ways of sharing that out to this community better, then I am all ears :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, fwiw, I'll be presenting at GopherCon (US) on my team's integration testing solution which incorporates this pkg! So if there are ways of sharing that out to this community better, then I am all ears :)

@sicoyle sure, please join our Slack so that we can have a more interactive conversation about it

It's amazing that you are sharing all that great stuff you are doing at this big conference. Congrats!

@sicoyle
Copy link
Contributor Author

sicoyle commented Sep 25, 2022

@kiview @mdelapenya I standardized the names that I searched and found in the docs to Testcontainers-go. Please let me know if you'd prefer any other changes!

`Reaper` in this package) removes containers/networks/volumes created by
Testcontainers-Go after a specified delay. It is a project developed by the
Testcontainers-go after a specified delay. It is a project developed by the
TestContainers organization and is used across the board for many of the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
TestContainers organization and is used across the board for many of the
Testcontainers organization and is used across the board for many of the

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, could you replace in this PR any reference to TestContainers? 🙏

- TestContainers
+ Testcontainers

I can find them at:

  • README.md
  • testing.go
  • gotest.md

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, probably the most important change 😅

Copy link
Member

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Thanks for taking care of the docs! Great improvement 👏

@mdelapenya
Copy link
Member

mdelapenya commented Sep 26, 2022

@sicoyle thanks for the PR, which has started a positive, internal discussion on naming things (sum-up: projects Vs repos) 😄

We are going to merge this PR as is, and we will eventually update the names ourselves, to avoid causing you more work playing ping pong with the names

Again, thanks for your work here.

@mdelapenya mdelapenya merged commit 6ee7f6a into testcontainers:main Sep 26, 2022
mdelapenya referenced this pull request in mdelapenya/testcontainers-go Dec 21, 2022
* main:
  Add system requirements parent docs page for podman and colima (#562)
  Support for cap-add/cap-drop (#555)
  fix container NetworkMode usage (#560)
  chore: use hashed versions of test-summary action (#556)
  chore: use container.State() function in tests (#543)
  Log docker server info (#548)
  docs: add docs regarding Colima usage (#547)
  chore: add emoji to breaking changes in release drafter (#542)
  chore: add CONTRIBUTING file (#539)
  issue #537 Rename the wait/multi.go file to wait/all.go (#541)
  docs: add a basic layout for wait strategies in docs (#536)
  docs: improve consistency and fix typos (#534)
  chore: do not skip test (#528)
  chore: include test flakiness in the release drafter (#535)
  chore: retire old versions of Go (#530)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Docs, docs, docs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants