-
-
Notifications
You must be signed in to change notification settings - Fork 127
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
Fix service type #382
Fix service type #382
Conversation
9935d8d
to
98d33bd
Compare
25038fc
to
bb9224e
Compare
.github/workflows/license-header.yml
Outdated
with: | ||
files: | | ||
packages/**/*.css | ||
packages/**/*.js | ||
packages/**/*.ts | ||
packages/**/*.tsx | ||
|
||
- name: Did things change? | ||
if: steps.changed-files.outputs.any_changed == 'true' | ||
shell: bash -l {0} | ||
run: | | ||
echo "Yes! Some things changed!" | ||
echo | git status --short --branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@afshin did you encounter an unexpected file being committed or is this just a fail-safe?
Note: to be sure to display the changes detected by the previous action, I would recommend using the following snippet:
lumino/.github/workflows/check-api-changes.yml
Lines 64 to 67 in d6a592c
run: | | |
for file in ${{ steps.api-changed.outputs.all_changed_files }}; do | |
echo "$file" | |
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No new files are committed, but for some reason the next job (the one that does the git push
) fails, presumably because there is nothing to push? I'm not 100% sure what is happening. I reverted my modifications to the CI job because I was just experimenting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got some similar problems with the API changes workflow. So I applied the same trick in dc2692d; i.e. forcing the commit then use the diff between HEAD and HEAD~1 to see if something changed.
bb9224e
to
fd1e34a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @afshin
For those of us trying to follow along: What exactly does this change do? What problems did the old code cause, and how is this a fix? |
Oh, I see now that this is a cleanup after #377. |
Right, this puts the signatures back to what they originally were. |
cleanup after #377
Fix API signatures