Skip to content

Turk/ci - #12

Open
turkosaurus wants to merge 39 commits into
mainfrom
turk/ci
Open

Turk/ci#12
turkosaurus wants to merge 39 commits into
mainfrom
turk/ci

Conversation

@turkosaurus

Copy link
Copy Markdown
Contributor

No description provided.

turkosaurus and others added 30 commits February 16, 2025 15:10
@katlukens

Copy link
Copy Markdown
Contributor

@turkosaurus will you look at flutter_test.yml and make sure it looks ok?

Comment thread .github/workflows/test_flutter.yml Outdated
Comment on lines +87 to +89
echo "Current directory: $(pwd)"
echo "Contents of current directory:"
ls -la

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Weird spacing and redundant. If you really want to keep it, clean it up.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

was just a debug. gone.

run: flutter build apk --release

- name: Build iOS (if applicable)
if: runner.os == 'macOS'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Does this not need the working dir option that android does?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It does, and has one now. :)

Comment thread .github/workflows/test_flutter.yml Outdated
run: |
if [ ! -f "$HOME/flutter/bin/flutter" ]; then
echo "Flutter SDK not found. Downloading..."
git clone https://github.com/flutter/flutter.git -b stable --depth 1 $HOME/flutter

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note the depth option here of 1 is just one ref -- no history is included, just what's needed to build the latest ref.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

noted. Seems like a good way to go. Is there a reason I would want a different depth?

Comment thread .github/workflows/test_flutter.yml Outdated
Comment on lines +62 to +63
- name: Debug Flutter SDK directory
run: ls -la $HOME/flutter/bin

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Replace "debug" with something more descriptive like "list," unless debug is meant to be temporary. In which case, delete it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

deleted.

@katlukens

Copy link
Copy Markdown
Contributor

@turkosaurus Will you review this and see if you think it is ready to merge to main? Can't ask for a review the normal way because this is your branch.

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.

2 participants