Skip to content
This repository has been archived by the owner on Mar 8, 2020. It is now read-only.

UAST diff #396

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

UAST diff #396

wants to merge 5 commits into from

Conversation

dennwc
Copy link
Member

@dennwc dennwc commented Apr 11, 2019

Supersedes #309. See the original PR for details.


This change is Reviewable

Signed-off-by: Wojciech Jabłoński <[email protected]>
@dennwc dennwc self-assigned this Apr 11, 2019
@dennwc dennwc mentioned this pull request Apr 11, 2019
Signed-off-by: Wojciech Jabłoński <[email protected]>
@creachadair
Copy link
Contributor

Before we merge this, we should go through and copy the important parts of the comment thread into the description here IMO.

@dennwc
Copy link
Member Author

dennwc commented Apr 11, 2019

Will add those comments as well. It still needs some work as well.

Denys Smirnov added 3 commits April 11, 2019 18:35
Signed-off-by: Denys Smirnov <[email protected]>
Signed-off-by: Denys Smirnov <[email protected]>
@dennwc
Copy link
Member Author

dennwc commented Apr 11, 2019

Rebased on the latest master, also cleaned up the code a bit. Ideally, need at least a few tests that check how the diff output looks like.

@bzz
Copy link
Contributor

bzz commented Apr 11, 2019

Nice job!

Shall we also briefly talk/present only the API that this functionality exposes to the potential users and get their feedback early on, so there are less chances of the need for invasive chances later on?

I know that @Jan21 was using alternative solution for extracting this dataset of structural diffs for every commit https://github.com/src-d/datasets/tree/master/StructuralCommitFeatures so it might make sense to ask @creachadair at next TL meeting to ping ML team to take a quick look, WDYT?

@dennwc
Copy link
Member Author

dennwc commented Apr 11, 2019

PR doesn't expose or define any API yet. API is still TBD as well as if we will use this specific diff implementation or not.

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

Successfully merging this pull request may close these issues.

4 participants