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

Execute GHAs only in maidsafe repository #189

Merged
merged 13 commits into from
Jan 21, 2021
Merged

Conversation

Thierry61
Copy link
Contributor

Forking Maidsafe repositories is a pain because they have GHA workflows that don’t work from other repositories du to secrets used in them. This generates errors and many mails with each push commands.

This PR, defined on qp2p as an example, solves this problem by executing actions only within Maidsafe repository but not in forked ones.

The solution is simple, add the following line to control execution of each action: if: ${{ github.repository_owner == 'maidsafe' }}.

Additionally, the bad indentation of tag_release.yml has been corrected.

@Thierry61 Thierry61 requested a review from a team as a code owner December 13, 2020 13:48
dirvine
dirvine previously approved these changes Dec 31, 2020
@Thierry61
Copy link
Contributor Author

Thierry61 commented Jan 3, 2021

As you have noticed, latest version of clippy doesn’t pass field_reassign_with_default requirement in src/peer_config.rs.
Suggested correction is the following:

diff --git a/src/peer_config.rs b/src/peer_config.rs
index d10a2e6..982e37f 100644
--- a/src/peer_config.rs
+++ b/src/peer_config.rs
@@ -46,11 +46,13 @@ pub fn new_our_cfg(
     our_key: quinn::PrivateKey,
 ) -> Result<quinn::ServerConfig> {
     let mut our_cfg_builder = {
-        let mut our_cfg = quinn::ServerConfig::default();
-        our_cfg.transport = Arc::new(new_transport_cfg(
-            idle_timeout_msec,
-            keep_alive_interval_msec,
-        ));
+        let our_cfg = quinn::ServerConfig {
+            transport: Arc::new(new_transport_cfg(
+                idle_timeout_msec,
+                keep_alive_interval_msec,
+            )),
+            ..Default::default()
+        };

         quinn::ServerConfigBuilder::new(our_cfg)
     };

Unfortunately this doesn’t compile because token_key, use_stateless_retry, retry_token_lifetime, accept_buffer, migration fields are pub(crate) (in struct quinn::generic::ServerConfig). To make it work they should be changed to pub.

Someone from Maidsafe should make a PR to quinn people to change this, unless you find a better solution.

@S-Coyle S-Coyle merged commit 87d8ae0 into maidsafe:master Jan 21, 2021
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.

6 participants