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

rename "example" dir to "_examples" #1734

Merged
merged 3 commits into from
Feb 8, 2022
Merged

rename "example" dir to "_examples" #1734

merged 3 commits into from
Feb 8, 2022

Conversation

frederikhors
Copy link
Collaborator

Should fix #1715.

I have:

  • Added tests covering the bug / feature (see testing)
  • Updated any relevant documentation (see docs)

@coveralls
Copy link

coveralls commented Nov 30, 2021

Coverage Status

Coverage decreased (-0.01%) to 75.215% when pulling 956e6f5 on frederikhors:rename-example-dir-to-_examples into a054373 on 99designs:master.

@StevenACoffman
Copy link
Collaborator

So I think you also need to change the github actions and documentation to match to get the test failures to go away?

@frederikhors
Copy link
Collaborator Author

@StevenACoffman what this test fails?

--- FAIL: TestRewriter (0.11s)
    --- FAIL: TestRewriter/default (0.11s)
        rewriter_test.go:27: 
            	Error Trace:	rewriter_test.go:27
            	Error:      	"[]" should have 2 item(s), but has 0
            	Test:       	TestRewriter/default

@duckbrain
Copy link
Contributor

@frederikhors It looks like internal/rewrite/rewriter_test.go should be reverted. It was referencing a file named example.go, which was not renamed (and is already in a directory named testdata, so it doesn't need to be).

@frederikhors
Copy link
Collaborator Author

@duckbrain done. What is the problem now?

@StevenACoffman can you please merge this as soon as we understand the remaining problem? I had to fix a lot of conflicts from the master branch.

@StevenACoffman
Copy link
Collaborator

Sure. If we can get it passing, I'd like to merge it sooner rather than later.

@frederikhors
Copy link
Collaborator Author

@StevenACoffman I do not understand what these 2 errors are.

@duckbrain
Copy link
Contributor

The Lint/Lint one probably needs running go generate on each of the examples. Because of the rename, go generate ./... will no longer generate the examples, so you'll need to refer to them individually.

The integration/federation one is likely referring to an old path in start.sh. need to look into it more.

@frederikhors
Copy link
Collaborator Author

I fixed lint step.

Now only this is the problem:

image

I cannot understand the reason!

@mtibben
Copy link
Member

mtibben commented Feb 8, 2022

.github/workflows/check-federation: line 7: ./start.sh: Permission denied

Were the permissions for start.sh changed? A chmod 755 start.sh should fix it

@frederikhors
Copy link
Collaborator Author

.github/workflows/check-federation: line 7: ./start.sh: Permission denied

Were the permissions for start.sh changed? A chmod 755 start.sh should fix it

Oh! Wow! I'm on Windows. Can you please chmod it?

@mtibben
Copy link
Member

mtibben commented Feb 8, 2022

hmmm you should be able to use WSL?

@frederikhors
Copy link
Collaborator Author

hmmm you should be able to use WSL?

I'm not really in that things... 😢

@duckbrain
Copy link
Contributor

@mtibben Good catch.

@frederikhors I don't use Windows, does git update-index --chmod=+x _examples/federation/start.sh do it?

Source https://www.scivision.dev/git-windows-chmod-executable/

Signed-off-by: Steve Coffman <[email protected]>
@StevenACoffman
Copy link
Collaborator

StevenACoffman commented Feb 8, 2022

I chmod'ed it for you.

@StevenACoffman
Copy link
Collaborator

StevenACoffman commented Feb 8, 2022

One concern is that it is worth highlighting in the directions for contributing code how to run all these checks locally. When they fail, it's not obvious from the github action failures what the problem is.

@frederikhors
Copy link
Collaborator Author

Can we merge now?

@StevenACoffman StevenACoffman merged commit 213a085 into 99designs:master Feb 8, 2022
@frederikhors frederikhors deleted the rename-example-dir-to-_examples branch February 8, 2022 23:29
StevenACoffman pushed a commit that referenced this pull request Feb 25, 2022
I believe the wrong link was changed in #1734. This fixes the links.
kbumsik added a commit to kbumsik/dd-trace-go that referenced this pull request Aug 17, 2022
Since v0.16.0, github.com/99designs/gqlgen/example/todo has been changed
to github.com/99designs/gqlgen/_examples/todo . And the current v0.14.0
was released last September. So update the module to v0.16.0 to make it
compatible with the lastest gqlgen.

Reference: 99designs/gqlgen#1734
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.

How to completely remove dataloaden from go.mod?
5 participants