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

Moving HikingProfile and CyclingProfile out of TripContentFilter #427

Merged
merged 9 commits into from
Oct 28, 2023

Conversation

trurlurl
Copy link
Contributor

The proposal is to move HikingProfile and CyclingProfile out of TripContentFilterGroup since they affect the search behavior and not the information to be returned about the trips found (filtering the information would be my interpretation of what TripContentFilter should do).

@trurlurl trurlurl added bug Something isn't working documentation labels Oct 26, 2023
@trurlurl trurlurl added this to the v2.0 milestone Oct 26, 2023
Copy link
Contributor

@ue71603 ue71603 left a comment

Choose a reason for hiding this comment

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

The proposed PR removes both Profiles from TripRefineRequest and TripChangeRequest.
Should I add them there as well?

@trurlurl
Copy link
Contributor Author

The proposed PR removes both Profiles from TripRefineRequest and TripChangeRequest. Should I add them there as well?

TripChangeRequestStructure - TripChangeParam - TripParams = TripParamStructure - TripMobilityFilter contains the profiles.

TripRefineRequest does not. I have a more fundamental question: We only have TripResult of the trip to be refined, this does not seem to include the initial TripParams - how is that sufficient to re-retrieve the trip?

trurlurl and others added 2 commits October 26, 2023 13:46
Moving HikingProfile and CyclingProfile out of TripContentFilterGroup
@ue71603 ue71603 force-pushed the hiking_and_cycling_profiles branch from 310f769 to 42f89ec Compare October 26, 2023 11:46
@trurlurl
Copy link
Contributor Author

The proposed PR removes both Profiles from TripRefineRequest and TripChangeRequest. Should I add them there as well?

TripChangeRequestStructure - TripChangeParam - TripParams = TripParamStructure - TripMobilityFilter contains the profiles.

TripRefineRequest does not. I have a more fundamental question: We only have TripResult of the trip to be refined, this does not seem to include the initial TripParams - how is that sufficient to re-retrieve the trip?

Proposal if it is not sufficient: add new OriginalTripParams to TripRefineParamStructure.

@ue71603 ue71603 self-requested a review October 26, 2023 15:20
ue71603
ue71603 previously approved these changes Oct 26, 2023
@ue71603 ue71603 requested a review from AndreasAtSBB October 26, 2023 15:20
@ue71603
Copy link
Contributor

ue71603 commented Oct 26, 2023

@herlitze pls check for the filtergroup I added to trip refinement. I hope this is ok for you as well.

@ue71603 ue71603 self-requested a review October 26, 2023 15:21
@ue71603
Copy link
Contributor

ue71603 commented Oct 28, 2023

@skinkie if ok, then pls approve and merge.

@skinkie skinkie merged commit 1985f0f into changes_for_v1.1 Oct 28, 2023
@skinkie skinkie deleted the hiking_and_cycling_profiles branch October 28, 2023 15:33
trurlurl added a commit that referenced this pull request Nov 3, 2023
skinkie pushed a commit that referenced this pull request Nov 7, 2023
* Update CHANGELOG.md

* #402, #404, #420, #425, #426, #427

---------

Co-authored-by: trurlurl <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working doc updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants