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

Refactor legacy hooks #1896

Merged
merged 10 commits into from
Mar 11, 2021
Merged

Refactor legacy hooks #1896

merged 10 commits into from
Mar 11, 2021

Conversation

wcekan
Copy link
Contributor

@wcekan wcekan commented Mar 10, 2021

Description

  • Multiple legacy life cycle annotations can be on same method.
  • Process "this" placeholder correctly in FilterTranslator and Operator
  • Restore functionality previously available with CommitChecks using runAtCommit method in Check.

Motivation and Context

Fixing issues found in migrating from 4.x to 5.x

How Has This Been Tested?

Unit testing. Local testing with own Elide project.

License

I confirm that this contribution is made under an Apache 2.0 license and that I have the authority necessary to make this contribution on behalf of its copyright owner.

@wcekan wcekan force-pushed the commit-checks branch 3 times, most recently from 1187a75 to 115aba7 Compare March 10, 2021 21:39
@wcekan wcekan force-pushed the commit-checks branch 2 times, most recently from 734a8ac to 20132db Compare March 11, 2021 10:43
*
* @return true to run at commit
*/
public boolean runAtCommit() {
Copy link
Member

Choose a reason for hiding this comment

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

Can we move this into Check as default method?

Then, in FilterExpressionCheck and UserCheck, override it to always return false and make the method final (so it cannot be overwritten).

@aklish aklish merged commit f1ed11a into master Mar 11, 2021
@aklish aklish deleted the commit-checks branch March 11, 2021 16:47
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