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

ngclient: review _preorder_depth_first_walk() #1398

Closed
jku opened this issue May 19, 2021 · 1 comment · Fixed by #1463
Closed

ngclient: review _preorder_depth_first_walk() #1398

jku opened this issue May 19, 2021 · 1 comment · Fixed by #1463
Assignees
Labels
Milestone

Comments

@jku
Copy link
Member

jku commented May 19, 2021

This is for the new client that's currently in experimental-client branch: but I'm hoping this issue is handled after the experimental client is merged to develop

We've not touched _preorder_depth_first_walk() or _visit_child_role() yet: they need some love:
First things:

  • type annotations would make the whole thing a lot easier to read and reason about
  • docstrings are old style
  • evaluate if some functionality could/should be in API (e.g. in DelegatedRole)

Then maybe:

  • maybe the code can be made simpler?
  • review the use of fnmatch(): it seems a bit suspicious (again using filesystem path functions for what I think are url fragments)
@jku jku changed the title experimental client: review _preorder_depth_first_walk() ngclient: review _preorder_depth_first_walk() May 25, 2021
@sechkova sechkova added the backlog Issues to address with priority for current development goals label May 26, 2021
@sechkova sechkova added this to the weeks22-23 milestone May 26, 2021
@sechkova sechkova assigned sechkova and unassigned sechkova May 26, 2021
@joshuagl joshuagl removed this from the weeks22-23 milestone Jun 9, 2021
@sechkova sechkova added this to the weeks24-25 milestone Jun 9, 2021
@joshuagl
Copy link
Member

joshuagl commented Jun 9, 2021

See also a request for clarification on the algorithm in the specification theupdateframework/specification#164

@sechkova sechkova self-assigned this Jun 9, 2021
@sechkova sechkova modified the milestones: weeks24-25, weeks26-27 Jun 23, 2021
@joshuagl joshuagl modified the milestones: weeks26-27, Sprint 4 Jul 7, 2021
@joshuagl joshuagl removed the backlog Issues to address with priority for current development goals label Jul 7, 2021
@joshuagl joshuagl modified the milestones: Sprint 4, Sprint 5 Jul 28, 2021
@jku jku closed this as completed in #1463 Aug 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants