ardanlabs / gotour

Apache License 2.0
100 stars 60 forks source link

Fix eng content tests; run tests via GitHub Actions #262

Closed alexandear closed 4 months ago

alexandear commented 4 months ago

The PR fixes the test that checks code examples in the _content/tour/eng directory and adds GitHub Actions workflow to run tests on each PR or push to the main branch.

Now, running go test -v ./... works flawlessly.

I have divided the PR into separate logical commits to facilitate review. Each commit corresponds to one logical change.

Changes:

alexandear commented 4 months ago

Follows #260


It's possible to check this PR locally via the GitHub CLI by executing the following command:

gh pr checkout 262 
ardan-bkennedy commented 4 months ago

This needs to merge to bill/testing

alexandear commented 4 months ago

It's possible to review this PR without merging it into bill/testing. For instructions, please visit Checking Out Pull Requests Locally.

ardan-bkennedy commented 4 months ago

Not sure why? I will need to merge it into main anyway. I can easily do that in this testing branch.

ardan-bkennedy commented 4 months ago

I can delete this branch and give you a new one to merge to?

alexandear commented 4 months ago

Not sure why? I will need to merge it into main anyway. I can easily do that in this testing branch.

I'm assuming we are using the GitHub flow. This flow includes the following steps:

  1. Forking a repository: I forked ardanlabs/gotour into alexandear/gotour.
  2. Making changes and creating a PR: I made changes in alexandear:fix-content-test and created this PR.
  3. Reviewing the PR by maintainers and adding comments: You need to review the PR either using the GitHub UI or by checking it out locally.
  4. Addressing comments: I need to address your comments on my PR.
  5. Merging the PR into the default branch by maintainer: After I have addressed the comments, you need to merge this PR into main by pressing the button "Merge". GitHub will automatically mark the PR as "Merged".
ardan-bkennedy commented 4 months ago

The PR is TOO BIG and it scares me. I have to review it and make sure nothing breaks. I need it in my testing branch I provided if you want to see this merged.

ardan-bkennedy commented 4 months ago

I created a new bill/testing2 branch for this PR.

alexandear commented 4 months ago

I reverted all changes related to local translations (they will be fixed later) and retained only the fixes for examples in the eng directory to simplify the review process.

Only two files require thorough review:

All other modifications add nobuild and norun build constraints to examples.

Also, the tests are passed on CI: https://github.com/ardanlabs/gotour/actions/runs/9681636341/job/26712671977?pr=262

ardan-bkennedy commented 4 months ago

I will NOT being merging this into main directly. You will need to merge this into bill/testing2 so I can gain access to the code directly and run my own test and review. If you are not willing to do this, I can't accept it. I apprecaite the time and effort you are putting into this. That is why I am asking you for this again.

alexandear commented 4 months ago

@ardan-bkennedy changed branch from main to bill/testing2.

ardan-bkennedy commented 4 months ago

I made a some minor refactoring changes. This code is nice and I was able to see what you did. Pull down these changes and you can give me a PR to main and I will accept it. Make sure you have your name in the CONTRIBUTORS file please.

alexandear commented 4 months ago

Thank you. Created #263.

My name already in the CONTRIBUTORS file.