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

[Merged by Bors] - feat(cdk): ensure readme is included in manifest #3230

Closed
wants to merge 11 commits into from
Closed

[Merged by Bors] - feat(cdk): ensure readme is included in manifest #3230

wants to merge 11 commits into from

Conversation

EstebanBorai
Copy link
Contributor

Ensures the README is included into the ipkg

@EstebanBorai EstebanBorai linked an issue May 5, 2023 that may be closed by this pull request
CDK: Ensure binaries and readme are included in tarball #3197
Closed
morenol
morenol reviewed May 5, 2023
View reviewed changes
crates/cdk/src/publish.rs Outdated Show resolved Hide resolved
@EstebanBorai EstebanBorai force-pushed the 3197-cdk-ensure-binaries-and-readme-are-included branch from d44c327 to 94c9f92 Compare May 5, 2023 20:46
@EstebanBorai EstebanBorai requested a review from morenol May 5, 2023 21:26
morenol
morenol reviewed May 5, 2023
View reviewed changes
crates/cdk/src/publish.rs Outdated Show resolved Hide resolved
@morenol
Copy link
Contributor

morenol commented May 5, 2023

Could you update

fluvio/.github/workflows/connector-publish.yml

Line 8 in 3d212b6

package-name:

We need to add a new input parameter and properly handle it. That way we can update our workflow in our connector repos to specify where the readme is

sehz
sehz requested changes May 5, 2023
View reviewed changes
crates/cdk/src/publish.rs Outdated Show resolved Hide resolved
@EstebanBorai EstebanBorai requested review from sehz and morenol May 6, 2023 19:57
This was referenced May 6, 2023
Closed
Closed
Closed
Closed
Closed
sehz
sehz requested changes May 6, 2023
View reviewed changes
.github/workflows/connector-publish.yml Outdated Show resolved Hide resolved
crates/cdk/src/publish.rs Outdated Show resolved Hide resolved
crates/cdk/src/publish.rs Outdated Show resolved Hide resolved
crates/cdk/src/publish.rs Outdated Show resolved Hide resolved
@morenol
Copy link
Contributor

morenol commented May 8, 2023

There is a CDK test failing with # output : Error: No "README.md" file found Maybe we need to add a README to the connector test folder so it passes

@EstebanBorai EstebanBorai force-pushed the 3197-cdk-ensure-binaries-and-readme-are-included branch from 66a70a6 to 0af53e3 Compare May 10, 2023 15:49
@EstebanBorai EstebanBorai force-pushed the 3197-cdk-ensure-binaries-and-readme-are-included branch from 0af53e3 to a74f74c Compare May 10, 2023 15:49
@EstebanBorai
Copy link
Contributor Author

There is a CDK test failing with # output : Error: No "README.md" file found Maybe we need to add a README to the connector test folder so it passes

With this commit: b735e84 we won't have to provide a explicit value for README, we will assume that the README is in the CWD.

Which is the case where we run cdk from the same directory as Connector.toml for instance.

@EstebanBorai EstebanBorai requested review from morenol and sehz May 10, 2023 19:04
sehz
sehz approved these changes May 10, 2023
View reviewed changes
Copy link
Contributor

@sehz sehz left a comment

Choose a reason for hiding this comment

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

LGTM. Nice work!

@EstebanBorai
Copy link
Contributor Author

bors r+

@bors
Copy link

bors bot commented May 11, 2023

Timed out.

@EstebanBorai
Copy link
Contributor Author

bors retry

bors bot pushed a commit that referenced this pull request May 11, 2023
@EstebanBorai
feat(cdk): ensure readme is included in manifest (#3230)
980b21e 
Ensures the README is included into the `ipkg`
@bors
Copy link

bors bot commented May 11, 2023

Build failed:

@EstebanBorai
Copy link
Contributor Author

bors retry

bors bot pushed a commit that referenced this pull request May 11, 2023
@EstebanBorai
feat(cdk): ensure readme is included in manifest (#3230)
b24245b 
Ensures the README is included into the `ipkg`
@bors
Copy link

bors bot commented May 11, 2023

Pull request successfully merged into master.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title feat(cdk): ensure readme is included in manifest [Merged by Bors] - feat(cdk): ensure readme is included in manifest May 11, 2023
@bors bors bot closed this May 11, 2023
@EstebanBorai EstebanBorai mentioned this pull request May 11, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sehz sehz sehz approved these changes

@digikata digikata Awaiting requested review from digikata

@morenol morenol Awaiting requested review from morenol

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CDK: Ensure binaries and readme are included in tarball
3 participants
@EstebanBorai @morenol @sehz