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

Remove auto-generated code from source tree? #2

Closed
karlek opened this issue Sep 24, 2019 · 23 comments · Fixed by #41
Closed

Remove auto-generated code from source tree? #2

karlek opened this issue Sep 24, 2019 · 23 comments · Fixed by #41

Comments

@karlek
Copy link
Collaborator

karlek commented Sep 24, 2019

As a general rule, generated files do not belong in the source code repository. [...]
A few reasons for deviating from the general rule are

  • After generating them, the files are further modified by hand. In that case, they are not really generated files any more and thus belong in the VCS.

[1]

I generally agree with this view. However, as also mentioned we have to fix the bug from the golang grpc generator. But, when that bug is fixed I think it would be nice to remove the generated source from the tree. Any opinions?

[1]: https://softwareengineering.stackexchange.com/a/192115

@mewmew
Copy link
Contributor

mewmew commented Sep 24, 2019

👍

@mewmew
Copy link
Contributor

mewmew commented Sep 24, 2019

Oh, second consideration. Having generated code commited makes it Go-getable. So we should consider pros and cons also with this in mind.

@karlek
Copy link
Collaborator Author

karlek commented Sep 24, 2019

If it's going to be a polyglot project, it's not going to be go-gettable for long

@mewmew
Copy link
Contributor

mewmew commented Sep 24, 2019

It should perhaps be go-getable, crate-able (what's the Rustonic term?), etc :)

Haha, just realized, just for the sake of it, the Pippi project should use Python just to be pip-installable.

@karlek
Copy link
Collaborator Author

karlek commented Sep 24, 2019

Hahah yeah, sounds better than pipp-able

@karlek
Copy link
Collaborator Author

karlek commented Sep 24, 2019

Can we find the issue of the gRPC code generation bug so we can track it here? Or the status of the next library version you mentioned?

@karlek karlek closed this as completed in b42b20e Sep 25, 2019
@mewmew
Copy link
Contributor

mewmew commented Sep 25, 2019

Haha, bra timing.

Hittade precis https://github.com/golang/protobuf#packages-and-input-paths

@karlek
Copy link
Collaborator Author

karlek commented Sep 25, 2019

Håller på att fixa protoc i docker-imagen så vi klarar testerna igen 👍

@mewmew
Copy link
Contributor

mewmew commented Sep 25, 2019

Hmm, seems like the fix didn't quite work.

u@x1 ~/D/p/proto> tree
.
├── bin
│   ├── github.com
│   │   └── lapsang-boys
│   │       └── pippi
│   │           └── proto
│   │               └── bin
│   │                   └── bin.pb.go
│   └── __pycache__
│       ├── bin_pb2.cpython-37.pyc
│       └── bin_pb2_grpc.cpython-37.pyc

@mewmew
Copy link
Contributor

mewmew commented Sep 25, 2019

We get a github.com/lapsang-boys/pippi/proto/bin/github.com/lapsang-boys/pippi/proto/bin/bin.pg.go file generated, so the import path is duplicated.

@mewmew mewmew reopened this Sep 25, 2019
@karlek
Copy link
Collaborator Author

karlek commented Sep 25, 2019

🤔

@mewmew
Copy link
Contributor

mewmew commented Sep 25, 2019

Fanns en annan option source_relative kan kika om den har med saken att göra.

@mewmew
Copy link
Contributor

mewmew commented Sep 25, 2019

Hoppas det här inte är ett issue med Go modules (och att det kräver GOPATH).

@mewmew
Copy link
Contributor

mewmew commented Sep 25, 2019

@mewmew
Copy link
Contributor

mewmew commented Sep 25, 2019

ref: https://github.com/golang/protobuf/blob/1680a479a2cfb3fa22b972af7e36d0a0fde47bf8/regenerate.sh#L31

     protoc -I$dir --go_out=plugins=grpc,paths=source_relative:$dir $p

@karlek
Copy link
Collaborator Author

karlek commented Sep 25, 2019

It works!

@karlek
Copy link
Collaborator Author

karlek commented Sep 25, 2019

Ref: golang/protobuf#748

@mewmew mewmew closed this as completed in 3b3b6cd Sep 25, 2019
@mewmew
Copy link
Contributor

mewmew commented Sep 25, 2019

More autogenerated files left to purge.

  • cmd/pippi/frontend/src/wailsbridge.js

This was discovered since #4 had more line additions than deletions. A simple sed replace should not have that effect.

2019-09-25-230334_244x73_scrot

Edit: lets keep this issue open until we know that all files in the repo are manually created.

@mewmew mewmew reopened this Sep 25, 2019
@karlek
Copy link
Collaborator Author

karlek commented Sep 28, 2019

Found another one!

$ find src/ -type f | grep -i -P "\.spec\."
src/app/loader.service.spec.ts
src/app/disassembly/disassembly.component.spec.ts
src/app/loader/loader.component.spec.ts
src/app/binary.service.spec.ts
src/app/control-bar/control-bar.component.spec.ts
src/app/hexdump/hexdump.component.spec.ts
src/app/app.component.spec.ts
src/app/disassembly.service.spec.ts
src/app/select-id/select-id.component.spec.ts
src/app/id.service.spec.ts

@mewmew
Copy link
Contributor

mewmew commented Sep 28, 2019

Shall we add a .gitignore for *.spec.tc?

@karlek
Copy link
Collaborator Author

karlek commented Sep 28, 2019

Well, either that or actually start writing tests haha

@mewmew
Copy link
Contributor

mewmew commented Sep 28, 2019

actually start writing tests haha

Hehe, both would result in a green master :p

@karlek
Copy link
Collaborator Author

karlek commented Sep 29, 2019

True true 😁

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 a pull request may close this issue.

2 participants