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

[BEAM-9301] Check in beam-linkage-check.sh #10841

Merged
merged 2 commits into from Feb 13, 2020

Conversation

suztomo
Copy link
Contributor

@suztomo suztomo commented Feb 12, 2020

#10769 (comment)

This is a script to compare linkage errors (checkJavaLinkage task in the root gradle project) between PR's branch and master branch.

This is a temporary solution before Linkage Checker implements exclusion rules (BEAM-9206).


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) and mention them in a comment (R: @username).
  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • Update CHANGES.md with noteworthy changes.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- Build Status --- --- Build Status
Java Build Status Build Status Build Status Build Status
Build Status
Build Status
Build Status Build Status Build Status
Build Status
Build Status
Python Build Status
Build Status
Build Status
Build Status
--- Build Status
Build Status
Build Status
Build Status
--- --- Build Status
XLang --- --- --- Build Status --- --- ---

Pre-Commit Tests Status (on master branch)

--- Java Python Go Website
Non-portable Build Status Build Status
Build Status
Build Status Build Status
Portable --- Build Status --- ---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

@suztomo
Copy link
Contributor Author

suztomo commented Feb 12, 2020

R: @iemejia
CC: @lukecwik

@@ -0,0 +1,99 @@
#!/bin/bash
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@iemejia I tried with zsh compatible shell script, but it's not trivial. Therefore this relies on bash.

Copy link
Member

Choose a reason for hiding this comment

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

No issues, the shebang should cover it so ok for me

Copy link
Member

@iemejia iemejia left a comment

Choose a reason for hiding this comment

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

LGTM Nice!

@iemejia iemejia merged commit 7b748f2 into apache:master Feb 13, 2020
@iemejia
Copy link
Member

iemejia commented Feb 13, 2020

Thanks it will be really useful to have this here so we can evolve it together if needed.

@iemejia
Copy link
Member

iemejia commented Feb 13, 2020

Oups first suggestion of improvement detect if master is checked eagerly to not waste time before failing

@suztomo
Copy link
Contributor Author

suztomo commented Feb 14, 2020

master is checked eagerly

What is it?

@iemejia
Copy link
Member

iemejia commented Feb 14, 2020

In the last part of the script we do git checkout master just to compare, however if master is already checked out in a different which can happen if you use git worktrees the script breaks losing all the previous work.
I think we can avoid the validation and make the script more robust by not checking master but the hash that corresponds to master. I opened #10865 to tackle this, PTAL @suztomo

@suztomo
Copy link
Contributor Author

suztomo commented Feb 14, 2020

Thank you for explanation.

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

Successfully merging this pull request may close these issues.

None yet

2 participants