Skip to content

Fix deps include dirs listing #20

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

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

Tomcat-42
Copy link
Contributor

@Tomcat-42 Tomcat-42 commented Nov 1, 2023

This pull request:

@Tomcat-42 Tomcat-42 requested a review from a team as a code owner November 1, 2023 20:53
@awakecoding
Copy link
Contributor

@Tomcat-42 I'll look into getting the secret added for the publishing workflow (our DevOps team is busy this week with our major release, so it may go to next week). Is publishing workflow limited to the master branch? We'd need to add a GitHub environment with the secret in, and restrict how it can be used. Ideally we'd still need to trigger the workflow ourselves, on the master branch only

@Tomcat-42
Copy link
Contributor Author

Tomcat-42 commented Nov 2, 2023

@Tomcat-42 I'll look into getting the secret added for the publishing workflow (our DevOps team is busy this week with our major release, so it may go to next week). Is publishing workflow limited to the master branch? We'd need to add a GitHub environment with the secret in, and restrict how it can be used. Ideally we'd still need to trigger the workflow ourselves, on the master branch only

The publish job should only run on tagged master now

@awakecoding
Copy link
Contributor

@Devolutions/devops we'll need an environment with the secret "CARGO_REGISTRY_TOKEN" for the publishing workflow, using the following settings in crates.io:

image

@awakecoding
Copy link
Contributor

@Tomcat-42 I'm tagging my @CBenoit - for the publishing workflow, our GitHub organization blocks external GitHub Actions like those used in the current workflow. I know Benoit worked on custom GitHub Actions, but we'd need a bit more time to do this properly with the limitations we have in place. I would suggest splitting the code changes from this pull request so they can be merged quickly, leaving us more time to set up a publishing workflow.

@Tomcat-42 Tomcat-42 force-pushed the fix/deps_include_dirs branch from a661f93 to b781876 Compare November 2, 2023 19:12
@Tomcat-42 Tomcat-42 changed the title Fix deps include dirs listing and add publishing action Fix deps include dirs listing Nov 2, 2023
@Tomcat-42
Copy link
Contributor Author

Ok, I Splited the GH Actions into #21

@awakecoding awakecoding merged commit 8dd8774 into Devolutions:master Nov 2, 2023
@Tomcat-42
Copy link
Contributor Author

About the BuildDependency::get_include_dirs changes, it is a trivial one. IMO it can be already merged. Can you make a new release please?

@Tomcat-42 Tomcat-42 deleted the fix/deps_include_dirs branch November 2, 2023 19:20
@CBenoit
Copy link
Member

CBenoit commented Nov 2, 2023

Thank you @Tomcat-42, FYI the new version is now available on crates.io

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

BuildDependency with multiple include directories doesn't allow for full usage.
3 participants