Skip to content

Conversation

@artmoskvin
Copy link
Collaborator

This PR addressed the bug report hide-org/hide-sdk#22. As noted in the report, the list files endpoint returns absolute paths. Using those paths with other endpoints results in failures because the endpoints expect relative paths. We use relative paths by design aiming to abstract the internal file organisation.

The absolute paths were coming from the file tree walk that we do when listing files. I fixed the issue by using a relative path for File creation. It was not trivial to decide when to extract the relative path. We make multiple calls in the tree walk function, some of them expect absolute path, other – relative. It would be great to review this block and to simplify it.

I also simplified test assertions which required reordering of the wanted files.

Copy link
Collaborator

@aleh-null aleh-null left a comment

Choose a reason for hiding this comment

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

LGTM, however I think we might hit the issue in Include, Exclude filters. One example is in include with double asterisk test.

@aleh-null aleh-null self-requested a review September 25, 2024 13:07
Copy link
Collaborator

@aleh-null aleh-null left a comment

Choose a reason for hiding this comment

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

Will resolve issues with Filter and gitingnore globs in later PRs.

@artmoskvin artmoskvin merged commit 768d841 into main Sep 25, 2024
@artmoskvin artmoskvin deleted the artm/list-files-bug branch September 25, 2024 13:19
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