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.

Sign up for GitHub

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

Jump to bottom

Remove legacy install path overrides for old packages #839

Open
1 task
roborourke opened this issue Oct 16, 2023 · 4 comments · May be fixed by #840
Open
1 task

Remove legacy install path overrides for old packages #839

roborourke opened this issue Oct 16, 2023 · 4 comments · May be fixed by #840
Labels
bug Existing functionality isn't behaving as expected should have Should be done, medium priority for now

Comments

@roborourke
Copy link
Contributor

roborourke commented Oct 16, 2023 •
edited by kovshenin

With removal of some modules and potentially more in future there is a list of legacy packages that Altis will override the installation path for. This should be seen as a bug because it's unexpected and hard to debug behaviour.

https://github.com/humanmade/altis-core/blob/master/inc/composer/class-override-installer.php#L55-L92

The list should be updated and backported, there may be some other packages to remove depending on the Altis version.

Acceptance criteria

  • Review, test and merge associated PR
@roborourke roborourke added the bug Existing functionality isn't behaving as expected label Oct 16, 2023
roborourke added a commit that referenced this issue Oct 16, 2023
@roborourke
Remove the legacy package list for Altis install path overrides
d1629df 
The legacy list was for Composer v1 and the transition between the 2. I think we can ditch this to avoid errors in future where packages are unintentionally forced into the vendor directory.

Fixes #839
@roborourke roborourke linked a pull request Oct 16, 2023 that will close this issue
Open
@mikelittle mikelittle added to refine Issues needing refinement. should have Should be done, medium priority for now and removed to refine Issues needing refinement. labels Oct 16, 2023
@mikelittle
Copy link
Contributor

No longer needed

  • humanmade/delegated-oauth (removed in v14)
  • altis/aws-analytics (removed in v17)
  • altis/experiments (not needed as far back as 13)
  • humanmade/meta-tags (not needed as far back as 13)

@roborourke
Copy link
Contributor Author

@mikelittle I made a PR already for this - I think the legacy packages list can just be removed, it was only to support composer v1. May need to remove Composer v1 from the product-dev test matrix too as v2 has been out for some time now.

@roborourke
Copy link
Contributor Author

roborourke commented Oct 18, 2023 •
edited

The particular issue we have on a project right now is the removal of humanmade/hm-gtm in v17 that was part of the Analytics module and not directly mentioned in the upgrade guide. We're working around it by checking both possible install locations before including it.

@kovshenin kovshenin removed the to refine Issues needing refinement. label Mar 13, 2024
@kovshenin
Copy link
Contributor

Hey team! Please add your planning poker estimate with Zenhub @ferschubert-hm @jerico @mikelittle @wisyhambolu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Existing functionality isn't behaving as expected should have Should be done, medium priority for now
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove the legacy package list for install path overrides humanmade/altis-core
3 participants
@roborourke @kovshenin @mikelittle