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 composer dependency listing for built branches #14900

Merged
merged 3 commits into from
Mar 18, 2020
Merged

Remove composer dependency listing for built branches #14900

merged 3 commits into from
Mar 18, 2020

Conversation

LeoColomb
Copy link
Contributor

@LeoColomb LeoColomb commented Mar 5, 2020 •
edited by jeherve
Loading

Fixes #13497

Changes proposed in this Pull Request:

See my comments inline for detail, and please let me know if there is a better way!

Testing instructions:

N/A

Proposed changelog entry for your changes:

  • Not sure it's really needed.

🚀

@jetpackbot
Copy link

jetpackbot commented Mar 5, 2020 •
edited
Loading

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: April 7, 2020.
Scheduled code freeze: March 31, 2020

Generated by 🚫 dangerJS against b9f6b64

@LeoColomb LeoColomb marked this pull request as ready for review March 5, 2020 19:09
@LeoColomb LeoColomb requested a review from a team March 5, 2020 19:09
@jeherve jeherve added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Type] Bug When a feature is broken and / or not performing as intended Build [Focus] Jetpack DNA labels Mar 6, 2020
@jeherve jeherve added this to the 8.4 milestone Mar 6, 2020
jeherve
jeherve requested changes Mar 6, 2020
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

Thank you! I left a few comments below.

Right now, we're left with modified files once the script is done running. It may be good to add something like git checkout -- composer.json after cd $DIR in the cleaning up step.

tools/build-release-branch.sh Outdated Show resolved Hide resolved
tools/build-release-branch.sh Outdated Show resolved Hide resolved
tools/build-release-branch.sh Outdated Show resolved Hide resolved
@jeherve jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Mar 6, 2020
@LeoColomb LeoColomb requested a review from jeherve March 8, 2020 22:38
@jeherve jeherve self-assigned this Mar 10, 2020
jeherve
jeherve approved these changes Mar 10, 2020
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This seems to work well in my tests. Merging. Thanks again for the contribution!

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! [Status] Ready to Merge Go ahead, you can push that green button! labels Mar 10, 2020
jeherve
jeherve requested changes Mar 10, 2020
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

Do you think you could rebase this PR to include some of the latest changes we've made to our end to end tests? Right now the end to end tests fail for this PR, so we can't merge it as is.

Thank you!

@LeoColomb
Copy link
Contributor Author

@jeherve Could you just use the update button? Would much more handy 🙂

@jeherve
Copy link
Member

jeherve commented Mar 17, 2020

True, I've rebased now.

@brbrr Could you take another look at that test? Thank you!

@jeherve jeherve added [Status] Blocked / Hold and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Mar 17, 2020
@brbrr
Copy link
Contributor

brbrr commented Mar 18, 2020

#15031 should disable E2E tests for forks. Rebase should fix the issue

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Blocked / Hold labels Mar 18, 2020
jeherve
jeherve previously approved these changes Mar 18, 2020
View reviewed changes
@jeherve jeherve removed the [Status] Ready to Merge Go ahead, you can push that green button! label Mar 18, 2020
@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Blocked / Hold labels Mar 18, 2020
@jeherve jeherve merged commit 22eb790 into Automattic:master Mar 18, 2020
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Mar 18, 2020
LeoColomb referenced this pull request Jun 12, 2020
@dereksmart
New build
e0fde56
@jeherve jeherve mentioned this pull request Jul 6, 2020
Closed
@jeherve jeherve mentioned this pull request Feb 3, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeherve jeherve jeherve approved these changes

Assignees

@jeherve jeherve

Labels
Build [Focus] Jetpack DNA [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Milestone
8.4
Development

Successfully merging this pull request may close these issues.

Incompatibility with a WP Composer installation
5 participants
@LeoColomb @jetpackbot @jeherve @brbrr @matticbot