Skip to content

Conversation

@jamesxu123
Copy link
Member

@jamesxu123 jamesxu123 commented Jun 21, 2021

Tickets:

List of changes:

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • New release
  • This change requires a documentation update

How has this been tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A
  • Test B

Test Configuration:

Firmware version:
Hardware:
Toolchain:
SDK:

Questions for code reviewers?

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • Listed change(s) in the Changelog
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have made corresponding changes to the documentation
  • Any dependent changes have been merged and published in downstream modules

@jamesxu123 jamesxu123 temporarily deployed to testing June 21, 2021 04:13 Inactive
@jamesxu123 jamesxu123 linked an issue Jun 21, 2021 that may be closed by this pull request
@jamesxu123 jamesxu123 self-assigned this Jun 21, 2021
@krubenok krubenok marked this pull request as ready for review June 21, 2021 04:21
@krubenok krubenok self-requested a review June 21, 2021 04:25
@krubenok
Copy link
Member

Before merging this and letting Dependabot loose on at least a full year of stale dependencies, I'd suggest updating as many as you can in one shot first to give it a head start. Since it produces a PR for every single out of date dependency, it'll create a lot of noise we can solve faster+easier in one shot.

- package-ecosystem: "npm" # See documentation for possible values
directory: "/" # Location of package manifests
schedule:
interval: "daily"
Copy link
Member

Choose a reason for hiding this comment

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

There is a conversation to be had around weekly vs. daily. IMO - daily is fine if the Auto-merging Github Action I mentioned in the issue is implemented and we trust the CI/CD enough.

Edit: Issue #702

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the auto-merge is something to be explored and it would make things a lot easier. However, we're in the middle of making a lot of changes to the test suite to speed it up so it's probably safer if we go with weekly + manual review for now.

If everything goes well, making it automated would be a great future addition and I'll add it to the dev road map for the year.

@krubenok krubenok marked this pull request as draft June 21, 2021 04:30
@jamesxu123 jamesxu123 temporarily deployed to testing June 21, 2021 17:52 Inactive
@jamesxu123 jamesxu123 temporarily deployed to testing June 21, 2021 20:52 Inactive
@jamesxu123
Copy link
Member Author

Before merging this and letting Dependabot loose on at least a full year of stale dependencies, I'd suggest updating as many as you can in one shot first to give it a head start. Since it produces a PR for every single out of date dependency, it'll create a lot of noise we can solve faster+easier in one shot.

Ran npm update and npm audit fix. There was a minor change with Sendgrid but it seems to be all good now after a quick fix.

@jamesxu123 jamesxu123 marked this pull request as ready for review June 21, 2021 20:56
@krubenok
Copy link
Member

krubenok commented Jun 21, 2021

Should the auto-merging action be done at the same time, or do you want to leave that for later? Missed the other comment, sounds good.

Also, a similar setup should probably be implemented on the dashboard repo.

@jamesxu123 jamesxu123 merged commit 12f9fbe into dev Jun 21, 2021
@jamesxu123 jamesxu123 deleted the feature/hac-34-add-dependabot branch June 21, 2021 23:18
logan-r pushed a commit that referenced this pull request Nov 29, 2021
* Create dependabot.yml

* chore: update dependencies

* fix: update EmailService.send to be compatible

* style: remove commented code

* fix: sendMultiple

* change dependabot weekly
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.

Dependabot?

3 participants