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

Epic - November Dependency Updates #5584

Closed
13 tasks done
joewhitsitt opened this issue Aug 17, 2022 · 16 comments
Closed
13 tasks done

Epic - November Dependency Updates #5584

joewhitsitt opened this issue Aug 17, 2022 · 16 comments
Assignees
Labels
epic large story pointing

Comments

@joewhitsitt
Copy link
Contributor

joewhitsitt commented Aug 17, 2022

We should try to review and merge in dependency updates on a monthly basis. Until this is automated more, here is a guide:

  • Issue summary should contain a list of the dependabot pulls in scope for reference, however, the actual work should be done in a single PR to minimize composer conflicts.
  • Dependencies related to our frontend build tools (not executed on the websites) are usually safe to merge as-is but might cause issue with other packages. Check travis build for messages related to blt frontend.
  • As a separate PR is created with the dependency update, the related dependabot version should be closed referencing the monthly PR.
  • Periodically check locked dependencies for new versions. Make notes about why a dependency was/is locked in the repo Readme.
  • Review the changelog for each version we are jumping to and the versions in-between. Make note of certain functionality in our codebase that is using the functionality for testing instructions later.
  • Use https://github.com/uiowa/uiowa#updating-dependencies as a guide to test for update hooks, configuration changes and commit. This command is helpful if there are a lot of configuration changes across feature/site splits: https://github.com/uiowa/uiowa/blob/main/blt/src/Blt/Plugin/Commands/ConfigSplitCommands.php
  • If a particular dependency is too big to go out along with others, split it off from the PR as a separate issue and introduce the "why" in parking lot.
  • Update hooks and requires cache rebuild type updates may require off-hours deployment.
  • Once this issue is completed, copy these notes to the next month's issue and introduce it in parking lot.

Updates in-scope

Possible locked dependency updates

@briand44 briand44 changed the title September Dependency Updates October Dependency Updates Sep 6, 2022
@briand44
Copy link
Contributor

briand44 commented Sep 6, 2022

Postponing this until October since we are updating core and php in Sept

@briand44 briand44 changed the title October Dependency Updates November Dependency Updates Oct 28, 2022
@pyrello pyrello added the large story pointing label Nov 10, 2022
@makurj
Copy link
Contributor

makurj commented Nov 14, 2022

I would like to drive/pair on this.

@makurj makurj self-assigned this Nov 14, 2022
@joewhitsitt joewhitsitt self-assigned this Nov 14, 2022
@joewhitsitt
Copy link
Contributor Author

joewhitsitt commented Nov 14, 2022

going to look into the locked dependencies while @makurj pulls together and reviews the dependabots.

This is an example of a frontend dependency that we just to need test whether our frontend process still works and there aren't percy issues I think #5644 it would not be in-scope for this issue

@pyrello
Copy link
Contributor

pyrello commented Nov 14, 2022

Need to set permissions based on #5609

I think that the intention behind #5609 was to prevent non-admins from accessing the direct add stuff, so actually I think just switching to that new version will just accomplish that without setting a permission.

@joewhitsitt joewhitsitt removed their assignment Nov 14, 2022
@joewhitsitt
Copy link
Contributor Author

Added the locked packages that could be in-scope for this issue. The rest are still waiting or need to remained locked. Noticed that acquia/blt-multisite could be updated to a stable version https://github.com/acquia/blt-multisite/releases/tag/v1.0.0

@joewhitsitt
Copy link
Contributor Author

If we have a separate PR (e.g. for drupal/diff) for example, here are some adjustments for the readme.md:

Package Reason
drupal/smart_date Need to wait for upstream issue to be part of stable release or we need to patch it #5664
acquia/blt-travis No stable release to pair with blt 13.5. See acquia/blt-travis#3

The modules that are updated as part of this issue could be removed from the table.

@joewhitsitt
Copy link
Contributor Author

@joewhitsitt Would we want to go to 1.10 instead of 1.8 https://www.drupal.org/project/menu_block/releases/8.x-1.10, primarily bug fixes

Looks like a good value add for php 8.1:
https://git.drupalcode.org/project/menu_block/-/compare/8.x-1.7...8.x-1.10?from_project_id=3161&straight=false

@pyrello pyrello added the epic label Nov 14, 2022
@pyrello pyrello changed the title November Dependency Updates Epic - November Dependency Updates Nov 14, 2022
@pyrello
Copy link
Contributor

pyrello commented Nov 15, 2022

@joewhitsitt Would we want to go to 1.10 instead of 1.8 https://www.drupal.org/project/menu_block/releases/8.x-1.10, primarily bug fixes

Looks like a good value add for php 8.1: https://git.drupalcode.org/project/menu_block/-/compare/8.x-1.7...8.x-1.10?from_project_id=3161&straight=false

Looks like @dependabot agrees: #5948

@pyrello
Copy link
Contributor

pyrello commented Nov 15, 2022

I'm working on the lb_direct_add update.

@pyrello
Copy link
Contributor

pyrello commented Nov 15, 2022

I was going to add the easysvg package update to #5949, but decided to hold off since I'm not sure how to test it and was hoping that @joewhitsitt could guide me on that.

@pyrello
Copy link
Contributor

pyrello commented Nov 16, 2022

I thought I might be able to sneak #5951 in, since it involves a dependency update, but it requires more work to transition our homespun view block filters to the new one provided by ctools_views. Therefore I'm not adding it to this list.

joewhitsitt added a commit that referenced this issue Nov 16, 2022
joewhitsitt added a commit that referenced this issue Nov 18, 2022
Co-authored-by: Sean Adams-Hiett <sean-adams-hiett@uiowa.edu>
@joewhitsitt joewhitsitt self-assigned this Nov 21, 2022
@joewhitsitt
Copy link
Contributor Author

Looks like all of the in-scope work is done on this. Credit to @pyrello and @makurj for this work.

Once this issue is completed, copy these notes to the next month's issue and introduce it in parking lot.

Based on recent discussions and how this issue was addressed, I am not going to create next month's issue. Team thoughts about how to work these in to our planned work without falling behind?

@pyrello
Copy link
Contributor

pyrello commented Nov 21, 2022

I think that the epic was helpful in keeping track of the updates to be done. Maybe we just need to rewrite the notes a bit?

@pyrello
Copy link
Contributor

pyrello commented Nov 21, 2022

We should try to review and merge in dependency updates on a monthly basis. Until this is automated more, here is a guide:

  • Review the outstanding dependency updates and add a list of current ones below.
  • Dependencies related to our frontend build tools (not executed on the websites) are usually safe to merge as-is but might cause issue with other packages. Check travis build for messages related to blt frontend.
  • Each @dependabot PR can be worked on separately. Each dependency PR you merge will cause merge conflicts with all the other ones. As long as you don't alter the branch (e.g. merge in main, push a commit), you can fix the merge conflicts by running @dependabot rebase.
  • If a @dependabot PR does require some type of modification (e.g. removing a patch, updating incompatible code), it is okay to make those changes directly in relevant PR. However, it is a good idea to wait until you have merged all the other "easy" dependency PRs first because you will lose the ability to use @dependabot rebase to have the PR's merge conflicts resolved.
  • Periodically check locked dependencies for new versions. Make notes about why a dependency was/is locked in the repo Readme.
  • Review the changelog for each version we are jumping to and the versions in-between. Make note of certain functionality in our codebase that is using the functionality for testing instructions later.
  • It can also be helpful to review the git diff comparison between the old version and the new one to see how extensive the code changes are and to get an idea of what things you might need to test.
  • Use https://github.com/uiowa/uiowa#updating-dependencies as a guide to test for update hooks, configuration changes and commit. This command is helpful if there are a lot of configuration changes across feature/site splits: https://github.com/uiowa/uiowa/blob/main/blt/src/Blt/Plugin/Commands/ConfigSplitCommands.php
  • Dependency PR's with update hooks or requiring a cache rebuild may require off-hours deployment. If this is the case, add the appropriate label to the PR.
  • Once this issue is completed, copy these notes to the next month's issue and introduce it in parking lot.

Updates in-scope

@joewhitsitt
Copy link
Contributor Author

I am okay with this. Thanks @pyrello for taking the time to tweak it.

@joewhitsitt
Copy link
Contributor Author

Created #5965. Closing this as #5951 should be addressed separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic large story pointing
Projects
None yet
Development

No branches or pull requests

4 participants