Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

larshp
Copy link
Member

@larshp larshp commented Feb 2, 2023

this is a start for extracting the git definitions to a new git definitions intf, zif_abapgit_git_definitions

zif_abapgit_definitions is getting too large, and its worth separating the responsibilities of the packages

this is just a start, trying to keep the amount of changes small, but its difficult

@larshp larshp changed the title refactor refactor, extract git definitions to git Feb 2, 2023
@larshp
Copy link
Member Author

larshp commented Feb 2, 2023

breaking lots of things, see cross check

@larshp larshp marked this pull request as ready for review February 3, 2023 06:33
@mbtools
Copy link
Member

mbtools commented Feb 3, 2023

You removed types completely and changed all the usage. That was a lot of work... Why not keep the types in the defs interface but point them to the git defs?

Keeping the following would reduce the impact to almost nothing:

INTERFACE zif_abapgit_definitions.

TYPES: 
  ty_sha1               TYPE zif_abapgit_git_definitions=>ty_sha1,              
  ty_file               TYPE zif_abapgit_git_definitions=>ty_file,               
  ty_files              TYPE zif_abapgit_git_definitions=>ty_files,
  ty_files_tt           TYPE zif_abapgit_git_definitions=>ty_files_tt,
  ty_file_signatures_tt TYPE zif_abapgit_git_definitions=>ty_file_signatures_tt,
  ty_comment            TYPE zif_abapgit_git_definitions=>ty_comment,
  ty_item_state         TYPE zif_abapgit_git_definitions=>ty_item_state,

There are a few more I would move or you might need in /git:

ty_chmod
ty_create
ty_commit
ty_commit_tt
ty_expanded
ty_expanded_tt
ty_ancestor

c_git_branch_type
c_head_name
c_git_branch
c_type
c_state
c_chmod
c_author_regex

@larshp
Copy link
Member Author

larshp commented Feb 3, 2023

well, having the same definitions multiple times, then it should be changed sometime anyhow?

yea, there is always more to move

this is just a start

Copy link
Member

@mbtools mbtools left a comment

Choose a reason for hiding this comment

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

moving on :-)

@larshp
Copy link
Member Author

larshp commented Feb 3, 2023

will see how much extra breakage there is 😁

@larshp larshp merged commit 773f2f7 into main Feb 3, 2023
@larshp larshp deleted the hvam/git020222 branch February 3, 2023 10:46
mbtools added a commit to abapGit/CI that referenced this pull request Feb 6, 2023
@mbtools mbtools mentioned this pull request Feb 6, 2023
@mbtools
Copy link
Member

mbtools commented Feb 6, 2023

#6029 (comment) somehow missed abapGit/CI (abapGit/CI#211) 😉

@larshp
Copy link
Member Author

larshp commented Feb 6, 2023

rule "unknown_types": false, is disabled in CI

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.