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

Git component #3

Closed
rnveach opened this issue Jun 5, 2017 · 6 comments
Closed

Git component #3

rnveach opened this issue Jun 5, 2017 · 6 comments
Assignees

Comments

@rnveach
Copy link
Member

rnveach commented Jun 5, 2017

Taken from #1 ,

We need to analyze git repository for changes in new commits in PR branch.

Notes:

  • We most ignore deleted files. Deleted checks and such can never be checked for changes as they are gone. Moved/Renamed files should stay to make sure there is no issue using them.
  • We must only grab files from src/main. Any changes in tests/it should be ignored.

Should we also grab the contents of the changes? Will they be needed?

Restricting files further by type should be done elsewhere as we will have to look at class hierarchy and such.

Luolc added a commit to Luolc/regression-tool that referenced this issue Jun 6, 2017
Luolc added a commit to Luolc/regression-tool that referenced this issue Jun 8, 2017
Luolc added a commit to Luolc/regression-tool that referenced this issue Jun 8, 2017
Luolc added a commit to Luolc/regression-tool that referenced this issue Jun 9, 2017
Luolc added a commit to Luolc/regression-tool that referenced this issue Jun 9, 2017
@rnveach
Copy link
Member Author

rnveach commented Jun 9, 2017

@Luolc Based on discussions in PR, we will not be dropping src/main or deleted files. We will try to pull them in for processing in #5 .

the possibility of grabing information from changes of UTs
The situation of "deleted files" is similar.

Will Git component do anything else?

Luolc added a commit to Luolc/regression-tool that referenced this issue Jun 11, 2017
Luolc added a commit to Luolc/regression-tool that referenced this issue Jun 11, 2017
@Luolc
Copy link
Contributor

Luolc commented Jun 11, 2017

@rnveach
I agree with your comment at #17 (review) that we would expends the fields in the future, including file path and changed lines.

From #17 (comment),

If you wish to separate this out more, than filtered files would be a parameter to this class.

Do you mean add some regex strings to this class? Would the filters only depend on path? I am not very sure now.

The mainly missing thing is the changed lines in GitChange, and maybe we could add them when we implement a generator which need them.

@rnveach
Copy link
Member Author

rnveach commented Jun 11, 2017

mean add some regex strings to this class?

That or full paths, but lets wait on this until we see a specific need.
I can see filtering on path and change type. I still think deleted files won't give us any benfits, but we'll wait and see.

maybe we could add them when we implement a generator which need them.

That sounds fine to me.

If you see nothing else to add to the core of the git component, than I think we can close this issue.

rnveach pushed a commit that referenced this issue Jun 11, 2017
@Luolc
Copy link
Contributor

Luolc commented Jun 11, 2017

@rnveach I think it is OK to close the issue now.

@rnveach rnveach closed this as completed Jun 11, 2017
@romani
Copy link
Member

romani commented Jun 25, 2017

just a note:
please do not be crazy with changes detection, it is better to grow its ability to detect changes with ability to process it.

@rnveach
Copy link
Member Author

rnveach commented Jun 25, 2017

it is better to grow its ability to detect

That is how we are breaking things apart.

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

No branches or pull requests

3 participants