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

Fix #39 - Slicer should allow filtering of a requirement based on the IU currently processed #41

Merged
merged 1 commit into from
Apr 21, 2022

Conversation

laeubi
Copy link
Member

@laeubi laeubi commented Apr 20, 2022

No description provided.

@mickaelistria
Copy link
Contributor

How is this supposed to be used? A custom implementation of Slicer that does override those methods should be provided?

@laeubi
Copy link
Member Author

laeubi commented Apr 20, 2022

A custom implementation of Slicer that does override those methods should be provided?

I think the most usual case is to extend the slicer, e.g PermissiveSlicer, or Tycho extend this class. The problem is that one can't get a back-pointer to the IU from a requirement so passing both is needed if one want to decide based on IU if a requirement is valid or not.

@laeubi
Copy link
Member Author

laeubi commented Apr 20, 2022

okay I might misunderstood that comment, there is only one extension in P2 of Slicer and I'd like to possible change/enhance this on a later change.

@laeubi
Copy link
Member Author

laeubi commented Apr 21, 2022

These are the relevant Typcho issues:

if there are no major concerns I'd like to ask that this is being merged , as this is an internal class we do not introduce new API and its quite obvious to provide the implementation with as much context as needed.

@akurtakov
Copy link
Member

I don't see any issue with this one. @mickaelistria is the one with deepest knowledge in the area but he would not be able to look into it in the next couple of weeks. Thus I think it's fine to merge.

@laeubi
Copy link
Member Author

laeubi commented Apr 21, 2022

Alright, would a committer be so kind to merge this then :-)

@HannesWell HannesWell merged commit cb68c11 into eclipse-equinox:master Apr 21, 2022
@HannesWell
Copy link
Member

Alright, would a committer be so kind to merge this then :-)

Done. :)
I agree with the estimation of both of you. Although I'm not very familiar with that part it looks like we don't damage much and I trust you that there is a good need for it.

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.

4 participants