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

Adds support to build promql parser in CI #7516

Closed

Conversation

Harkishen-Singh
Copy link
Contributor

@Harkishen-Singh Harkishen-Singh commented Jul 4, 2020

Signed-off-by: Harkishen-Singh harkishensingh@hotmail.com

Fixes: #7488

From pipeline

>> running check for promql-parser build
go get -u golang.org/x/tools/cmd/goyacc
go: found golang.org/x/tools/cmd/goyacc in golang.org/x/tools v0.0.0-20200702044944-0cc1aa72b347
goyacc -o promql/parser/generated_parser_temp.y.go promql/parser/generated_parser.y
conflicts: 2 shift/reduce
diff -I '^// Code generated by goyacc -o.*' -I '^//line.*' promql/parser/generated_parser_temp.y.go promql/parser/generated_parser.y.go
rm promql/parser/generated_parser_temp.y.go y.output

Signed-off-by: Harkishen-Singh <harkishensingh@hotmail.com>
@Harkishen-Singh
Copy link
Contributor Author

/cc @brian-brazil @roidelapluie

Signed-off-by: Harkishen-Singh <harkishensingh@hotmail.com>

remove manual install goyacc

Signed-off-by: Harkishen-Singh <harkishensingh@hotmail.com>

install goyacc manually

Signed-off-by: Harkishen-Singh <harkishensingh@hotmail.com>
Makefile Outdated
.PHONY: promql-parser
promql-parser:
@echo ">> running check for promql-parser build"
go get -u golang.org/x/tools/cmd/goyacc
Copy link
Member

Choose a reason for hiding this comment

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

somehow that would change the go mod?

Copy link
Member

Choose a reason for hiding this comment

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

I think goyacc should be installed in circleci config, not in the makefile

Signed-off-by: Harkishen-Singh <harkishensingh@hotmail.com>
Signed-off-by: Harkishen-Singh <harkishensingh@hotmail.com>
Signed-off-by: Harkishen-Singh <harkishensingh@hotmail.com>
@Harkishen-Singh
Copy link
Contributor Author

@roidelapluie can you give a look at why is the test failing, and a possible suggestion to fix it?

@roidelapluie roidelapluie self-assigned this Sep 21, 2020
@stale stale bot added the stale label Nov 20, 2020
@Harkishen-Singh
Copy link
Contributor Author

@roidelapluie any views?

@stale stale bot removed the stale label Nov 27, 2020
@stale stale bot added the stale label Jan 27, 2021
Base automatically changed from master to main February 23, 2021 19:36
@stale stale bot removed the stale label Feb 23, 2021
@stale stale bot added the stale label Apr 26, 2021
@stale stale bot removed the stale label Apr 26, 2021
@Harkishen-Singh
Copy link
Contributor Author

ping @roidelapluie

@stale stale bot added the stale label Jun 26, 2021
Copy link
Member

@codesome codesome left a comment

Choose a reason for hiding this comment

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

LGTM in general, but I have less context around this one. @roidelapluie could you give it another look since you had some comments?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ci: make sure promql/parser/generated_parser.y.go is reproducible
3 participants