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

Update the workflow file to follow the example of the main Comunica repository #155

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

surilindur
Copy link
Contributor

@surilindur surilindur commented Nov 26, 2024

This is a work-in-progress PR, so I will make it a draft. It is also based off #154 so the changelog might look strange.

This is now based on the master branch, and does not depend on other branches, so I made it an actual PR. Any feedback would be welcome. 🙂 I was going to enable the system tests on all three platforms, but they were all failing, maybe due to the server getting overloaded or something, so I left them as they were.

@surilindur surilindur force-pushed the chore/workflow-updates branch 2 times, most recently from 7932eaa to 9a7dfb8 Compare November 26, 2024 14:27
@surilindur surilindur marked this pull request as ready for review November 26, 2024 14:41
run: yarn install --frozen-lockfile --ignore-engines
- name: Build project
run: yarn run build
- name: Build project -- esModuleInterop true
Copy link
Member

Choose a reason for hiding this comment

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

Just like on base comunica, we need to keep this one as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tsconfig file of link traversal actually has the flag in it already, so this extra build step is not needed here. That is why I removed it.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, right, that's because of a dep on one of the inrupt libs.
Now, I would still keep it, because I want to remove that flag from the config when possible.
So we need to keep this in the CI, otherwise we may forget re-adding it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, it is back now, but in the tooling test step. That one felt like the correct place to add it.

@surilindur surilindur force-pushed the chore/workflow-updates branch from dd90751 to b219bd6 Compare December 4, 2024 10:55
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