-
Notifications
You must be signed in to change notification settings - Fork 6
Don't check file existence for entities not part of sync run #167
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
Conversation
60e472b to
873a67d
Compare
6ebce8d to
4b39435
Compare
4b39435 to
3fdb949
Compare
.github/workflows/build.yml
Outdated
| build: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 |
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.
please updat to v6 for checkoutg and check version of others actions
.github/workflows/release.yml
Outdated
| - run: pnpm publish --access=public --no-git-checks | ||
| if: ${{!github.event.release.prerelease}} | ||
| env: | ||
| NODE_AUTH_TOKEN: ${{ secrets.NPM_API_KEY }} |
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.
please remove node_auth_token ( I tihnk its only for release) and change it to oidc
src/modules/sync/validation.ts
Outdated
| export const validateSyncModelFolder = async (folderPath: string) => { | ||
| export const validateSyncModelFolder = async ( | ||
| folderPath: string, | ||
| entities: ReadonlySet<SyncEntityName>, |
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.
why is this set?
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.
Conceptually, I have to agree with Claude that it makes sense for this to be a readonly set. Having said that, I do agree with you that it is unnecessary complication as we never have the data in this structure so I refactored the function to accept an array of entity names instead.
Motivation
Which issue does this fix? Fixes #
issue numberIf no issue exists, what is the fix or new feature? Were there any reasons to fix/implement things that are not obvious?
Checklist
How to test
If manual testing is required, what are the steps?