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

Rewrite Version Selection, Logging & more #147

Closed
wants to merge 0 commits into from

Conversation

christophermancini
Copy link

@christophermancini christophermancini commented Aug 29, 2019

We are still testing these changes internally and expect further changes as we work out kinks, but creating the PR now so that we can open up dialog about merging these changes upstream.

The bulk of the work in this PR was captured in digitalocean/github-pr-resource#1

@christophermancini christophermancini requested a review from a team as a code owner August 29, 2019 21:46
@christophermancini christophermancini changed the title WIP: Rewrite Version Selection, Logging & more [skip ci] Rewrite Version Selection, Logging & more Aug 30, 2019
@christophermancini
Copy link
Author

@itsdalmo I just merged the latest features into our fork, makes this a good time to review my work before more conflicts are introduced.

@itsdalmo
Copy link
Contributor

itsdalmo commented Sep 4, 2019

Hi @christophermancini and thank you for the PR! There seems to be a lot of useful features in here, but it's pretty hard to review since it's such a big PR with lots of different features. Are there any particular features in this PR you'd want to see merged in upstream that we could focus on first? 🤔

@christophermancini
Copy link
Author

@itsdalmo I completely understand that this is a massive PR. My original goal was to refactor the version selection, but then it was rabbit hole after rabbit hole.

My thoughts are, it might be best to merge this to a new branch upstream, maybe an v2-RC? This way, folks in the community can start playing with it and provide feedback, if all looks good, move forward with it in the future. I would be happy to continue to keep it merge-able for the foreseeable future.

We are currently using it internally at DO and making adjustments as we go to get it tuned.

@christophermancini
Copy link
Author

christophermancini commented Sep 7, 2019

At this point the resource is working quite well. There is one known edge case where if a PR had more than one event in a very short time, the resource may produce two versions for that PR when it should have only produced one. That being said, it does not appear to have missed any PRs over the last few weeks averaging 500 builds per day on one of our pipelines that it is implemented on.

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.

None yet

2 participants