-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
Add Renovate to Ivy Integration tests #28121
Conversation
//cc @alexeagle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks promising. I'll let Alex review/approve.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a little troubled by the inability to compute what branches to watch but not enough to block landing this and seeing how it goes. Let's start with master
only, eh?
The problem with watching the patch branch, is that we would need to update this list of branches each time we change the patch branch. This would be yet another step in the caretaker's duties. But I could see that we could also automate this update in a future release script.
Pete, would it be crazy to have a "patch" or "latest" tracking branch on
the upstream repo and reset this branch at every rc.0? That way all of our
scripts can rely on a stable name for this branch and we don't need to
manually update stuff or compute the branch name based on npm tags.
Reseting the branch is not great but it might be a worthy trade off.
Could we consider this in your releasing and branching doc?
…On Thu, Jan 24, 2019, 8:22 AM Pete Bacon Darwin ***@***.*** wrote:
***@***.**** approved this pull request.
I am a little troubled by the inability to compute what branches to watch
but not enough to block landing this and seeing how it goes. Let's start
with master only, eh?
The problem with watching the patch branch, is that we would need to
update this list of branches each time we change the patch branch. This
would be yet another step in the caretaker's duties. But I could see that
we could also automate this update in a future release script.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#28121 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AANM6JmGFrXNCeOUA25-6NIMNRhVlSxxks5vGd3PgaJpZM4Z9yzw>
.
|
@IgorMinar I think you might have a good idea here. It would indeed make renovate much more usable, and also make the merging of PRs more straightforward for the caretaker - since they don't have to think about what version we are on; just merge to the branch(es) that matches the PR target(s). And releases are much less common than merges so we should aim to simplify the more common case. |
"enabled": false | ||
} | ||
}, | ||
"packageRules": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are comments allowed in this file? it would be nice to answer some questions inline (like does this section mean that we only update these three package groups?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unfortunately not, it's not possible to add comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we use a .js
file instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah though, it has to be named config.js
in the root of project. Not sure how you all feel about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to this, we can use an arbitrary name via RENOVATE_CONFIG_FILE
, but this is still less than ideal.
That is a very weird default name. Maybe renovate would want to change it if we asked politely 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t know why they didn’t bandit renovate-config.js
by default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexeagle what do you think of the above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather keep json with no comments, we should add README content somewhere about how renovate works and how to operate it though. Can be another PR
This will be important to keep the CLI / Framework changes in sync and not to have size regressions. TOOL-582 #resolve
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This will be important to keep the CLI / Framework changes in sync and not to have size regressions.
TOOL-582