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

Add initial Tilt support #297

Merged
merged 1 commit into from
Jul 28, 2023
Merged

Add initial Tilt support #297

merged 1 commit into from
Jul 28, 2023

Conversation

ncdc
Copy link
Member

@ncdc ncdc commented Jul 25, 2023

Description

Add initial Tilt support

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

@codecov
Copy link

codecov bot commented Jul 25, 2023

Codecov Report

Merging #297 (142f2ec) into main (669c451) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #297   +/-   ##
=======================================
  Coverage   80.69%   80.69%           
=======================================
  Files          18       18           
  Lines         803      803           
=======================================
  Hits          648      648           
  Misses        112      112           
  Partials       43       43           
Flag Coverage Δ
e2e 55.79% <ø> (ø)
unit 77.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

.gitignore Show resolved Hide resolved
This is typically a short as:

```shell
tilt up
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Does this need to be run inside the git repo?
  2. Does this "take over" the shell/terminal? (i.e. recommend opening another terminal?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes and yes

tilt.md Outdated Show resolved Hide resolved
tilt.md Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
Here is the same workflow with Tilt:

1. Run `tilt up`
2. Modify the source code
Copy link
Contributor

Choose a reason for hiding this comment

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

Might want to emphasize "with your preferred editor", so people don't think you need to use the Tilt interface.

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 don't think it's likely people will be confused.

Tiltfile Outdated Show resolved Hide resolved
@ncdc
Copy link
Member Author

ncdc commented Jul 25, 2023

Updated, thanks

tilt.md Outdated Show resolved Hide resolved
Copy link
Contributor

@tmshort tmshort left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 25, 2023
Copy link
Contributor

@tmshort tmshort 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 need .dockerignore?

@ncdc
Copy link
Member Author

ncdc commented Jul 25, 2023

No because the context when building images in bin/linux

@tmshort
Copy link
Contributor

tmshort commented Jul 25, 2023

No because the context when building images in bin/linux

I see that now, it was a bit hard because of some inconsistencies in the Makefile (i.e. the use of docker vs $CONTAINER_RUNTIME).

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 28, 2023
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 28, 2023
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 28, 2023
@tmshort
Copy link
Contributor

tmshort commented Jul 28, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 28, 2023
Signed-off-by: Andy Goldstein <andy.goldstein@redhat.com>
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 28, 2023
@tmshort
Copy link
Contributor

tmshort commented Jul 28, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 28, 2023
@ncdc ncdc merged commit b5e9abf into operator-framework:main Jul 28, 2023
9 checks passed
@ncdc ncdc deleted the tilt branch August 4, 2023 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants