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

[TINKERPOP-2927] Relax steps extensibility strictness #2027

Conversation

porunov
Copy link
Contributor

@porunov porunov commented Apr 18, 2023

In this commit the next changes are done for the Step classes:

  • Make private fields as protected
  • Allow to overwride any method by removing final keyword
  • Allow to extend any Step class by removing final keyword
  • Add getter methods for some fields which might be useful for Step information retrieval

Issue: https://issues.apache.org/jira/browse/TINKERPOP-2927
Discussion: https://lists.apache.org/thread/vjbjh29kwjhd5lcmkqqqqrhw7rw2ynh9

In this commit the next changes are done for the Step classes:
- Make private fields as protected
- Allow to overwride any method by removing `final` keyword
- Allow to extend any Step class by removing `final` keyword
- Add getter methods for some fields which might be useful for Step information retrieval

https://issues.apache.org/jira/browse/TINKERPOP-2927

Signed-off-by: Oleksandr Porunov <alexandr.porunov@gmail.com>
@Cole-Greer
Copy link
Contributor

Thanks @porunov for submitting this PR. The changes all look valid to me, but opening all of this up is a big change to TinkerPop and needs further consideration. Could you make a [DISCUSS] thread on the dev maillist to ensure that the community is aligned behind this change?

@porunov
Copy link
Contributor Author

porunov commented Apr 19, 2023

Thanks @porunov for submitting this PR. The changes all look valid to me, but opening all of this up is a big change to TinkerPop and needs further consideration. Could you make a [DISCUSS] thread on the dev maillist to ensure that the community is aligned behind this change?

Thank you for the feedback. I opened the discussion thread here: https://lists.apache.org/thread/vjbjh29kwjhd5lcmkqqqqrhw7rw2ynh9

@codecov-commenter
Copy link

Codecov Report

Merging #2027 (173444a) into 3.6-dev (d0e02f1) will decrease coverage by 4.89%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##             3.6-dev    #2027      +/-   ##
=============================================
- Coverage      69.39%   64.51%   -4.89%     
=============================================
  Files            878       25     -853     
  Lines          42103     3793   -38310     
  Branches        5643        0    -5643     
=============================================
- Hits           29219     2447   -26772     
+ Misses         10891     1179    -9712     
+ Partials        1993      167    -1826     

see 853 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@porunov
Copy link
Contributor Author

porunov commented Apr 20, 2023

Marked this PR as draft for now until we have some consensus in the discussion thread.
I believe the following PRs should be merged instead as for now:
#2029
#2030
#2031

@Cole-Greer
Copy link
Contributor

Going to close this PR based on the discussion on the dev list.
https://lists.apache.org/thread/vjbjh29kwjhd5lcmkqqqqrhw7rw2ynh9

Additional steps can be opened up in the future but will be handled on a case by case basis. See dev list discussion for more details.

@Cole-Greer Cole-Greer closed this Jul 12, 2023
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 this pull request may close these issues.

3 participants