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.

Sign up for GitHub

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

Jump to bottom

Replace travis job with github action. #66

Merged
merged 3 commits into from Oct 4, 2020
Merged

Replace travis job with github action. #66

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
817105a
Replace travis job with github action.
jmrtt Oct 3, 2020
ec6aac8
Add action rule in Makefile to perform the same steps as github action.
jmrtt Oct 4, 2020
c6db89c
Add missing regenerate step.
jmrtt Oct 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
47 changes: 47 additions & 0 deletions .github/workflows/build.yaml
View file
Open in desktop
@@ -0,0 +1,47 @@
name: build

on: [push, pull_request]

jobs:
build:
runs-on: ubuntu-latest
env:
working-directory: ./src/github.com/awalterschulze/gographviz

steps:
- name: Set GOPATH
run: |
echo "##[set-env name=GOPATH;]$(dirname $GITHUB_WORKSPACE)/gographviz"
echo "##[add-path]$(dirname $GITHUB_WORKSPACE)/gographviz/bin"
shell: bash

- name: Setup Go
uses: actions/setup-go@v2
with:
go-version: '1.14'

- name: Checkout
uses: actions/checkout@v2
with:
fetch-depth: 1
path: src/github.com/awalterschulze/gographviz

- name: Install dependencies
run: make dependencies
working-directory: ${{env.working-directory}}

- name: Run regenerate
run: make regenerate
working-directory: ${{env.working-directory}}

- name: Run build
run: make build
working-directory: ${{env.working-directory}}

- name: Run checkers
run: make checkers
working-directory: ${{env.working-directory}}

- name: Run testing
run: make test
working-directory: ${{env.working-directory}}
10 changes: 0 additions & 10 deletions .travis.yml
View file
Open in desktop

This file was deleted.

33 changes: 26 additions & 7 deletions Makefile
View file
Open in desktop
@@ -1,16 +1,35 @@
regenerate:
.PHONY: help regenerate test dependencies build checkers action

# Prefer tools that we've installed
export PATH := $(HOME)/go/bin:$(PATH)

help:
@grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'

regenerate: ## Re-generate lexers and parsers and pass through goimports
Copy link
Owner

Choose a reason for hiding this comment

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

This looks great 👍

I do not see the regenerate step in github actions.
The git diff --exit-code is also used to check that the newest generated code was checked in.

Otherwise it looks amazing :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, missed regenerate. ☹️

One question, when I execute make regenerate, the created imports for errors and token are local paths. I was expecting import with github.com/jmrtt/gographviz or better yet with github.com/awalterschulze/gographviz. So I'm missing something here.

The import after running make regenerate looks like "home/roquette/gographviz/internal/token" and goimport gives an error as expected. What I'm doing wrong?

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 saw that is possible to specify the package in gocc command-line using -p. But it was being ignored since the output is going to ./internal due to https://github.com/goccmack/gocc/blob/65cbc60dd30507e085bed479695e11aaf6fe9c2e/internal/config/config.go#L184

So, I changed the makefile to pass the package as parameter and generate inside internal folder (cd internal; gocc -zip -p github.com/awalterschulze/gographviz/internal ../dot.bnf). Please check if this is the best way to perform this in my last commit.

With this, I updated the generated code and is possible to check using git diff --exit-code.

Last run of the action can be found in: https://github.com/jmrtt/gographviz/runs/1205227899

Copy link
Owner

Choose a reason for hiding this comment

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

Aha I think the problem is the GOPATH variable needs to be set.
I think gocc uses the GOPATH to generate the paths.

I think travis used to set this as part of its default go setup

Copy link
Owner

Choose a reason for hiding this comment

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

I think with setting the GOPATH we should be able to get away with the old gocc -zip -o ./internal/ dot.bnf which should then also remove the generated txt files, I hope at least.

Copy link
Owner

Choose a reason for hiding this comment

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

Yes it seems it uses the go/build package to figure out the default package
https://github.com/goccmack/gocc/blob/master/internal/config/config.go#L205

I think I still remember encouraging this idea.

Copy link
Owner

Choose a reason for hiding this comment

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

Thank you for putting in this much effort.
I think this work is needed and I do appreciate it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the all the information! 😄 I will revert my last commit and try setting GOPATH to have gocc -zip -o ./internal/ dot.bnf back. I'll let you know when have a new iteration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With GOPATH and correct location, only token.go was updated by regenerate. Generated txt files didn't appear.

So, we already have the regeneration step and github actions working correctly (needed to set GOPATH and working directory to have it working).

Last run of github actions with regenerate: https://github.com/jmrtt/gographviz/runs/1205882765

Copy link
Owner

Choose a reason for hiding this comment

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

Amazing work.

Well done, this looks great.

I think with time of not having a working CI, the token.go got outdated some how. So including in the pull request makes sense :)

go get github.com/goccmack/gocc
go install github.com/goccmack/gocc
gocc -zip -o ./internal/ dot.bnf
gocc -zip -o ./internal/ dot.bnf
find . -type f -name '*.go' | xargs goimports -w
go mod tidy

test:
test: ## Perform package tests
go test ./...

travis:
Copy link
Owner

Choose a reason for hiding this comment

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

Is it possible to change this to action instead of travis and then call this from the github action.
That way it is possible to reproduce the github action locally before pushing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a really good idea, I forgot about the possibility to run locally...

In order to have separated steps in github action, I will add different rules to Makefile (dependencies, build and checkers) and an action one that calls the others.

make regenerate
go build ./...
go test ./...
dependencies: ## Grab necessary dependencies for checkers
go version
go get golang.org/x/tools/cmd/goimports
go get github.com/kisielk/errcheck
go get -u golang.org/x/lint/golint
go mod tidy

build: ## Perform build process
go build .

checkers: ## Run all checkers (errcheck, gofmt and golint)
errcheck -ignore 'fmt:[FS]?[Pp]rint*' ./...
gofmt -l -s -w .
golint -set_exit_status
git diff --exit-code

action: dependencies regenerate build test checkers ## Run steps of github action
5 changes: 3 additions & 2 deletions Readme.md
View file
Open in desktop
Expand Up @@ -24,9 +24,10 @@ output := graph.String()
### Installation ###
go get github.com/awalterschulze/gographviz

### Tests ###
### Build and Tests ###

[![Build Status](https://github.com/awalterschulze/gographviz/workflows/build/badge.svg)](https://github.com/awalterschulze/gographviz/actions)

[![Build Status](https://travis-ci.org/awalterschulze/gographviz.svg?branch=master)](https://travis-ci.org/awalterschulze/gographviz)

### Users ###

Expand Down
7 changes: 0 additions & 7 deletions install-godeps.sh
View file
Open in desktop

This file was deleted.

60 changes: 60 additions & 0 deletions internal/token/token.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.