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.

Sign up for GitHub

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

Feature/require script consistency #1451

Merged
merged 5 commits into from
May 27, 2022
Merged

Feature/require script consistency #1451

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c4f845c
Feat: js action
Blu-J May 16, 2022
8b95ff2
fix: Change the source + add input
Blu-J May 17, 2022
b740313
require script is present during pack step iff any pkg procs are type…
ProofOfKeags May 17, 2022
238c1c6
warn if script is present but manifest does not require one
ProofOfKeags May 19, 2022
7245f25
cleanup after rebase
ProofOfKeags May 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
warn if script is present but manifest does not require one
  • Loading branch information
@ProofOfKeags
ProofOfKeags committed May 27, 2022
commit 238c1c6d8e00be2970913ebb430ea4befa493197
16 changes: 10 additions & 6 deletions backend/src/s9pk/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,14 +123,18 @@ pub async fn pack(#[context] ctx: SdkContext, #[arg] path: Option<PathBuf>) -> R
})
.scripts({
let script_path = path.join(manifest.assets.scripts_path()).join("embassy.js");
if manifest.package_procedures().any(|a| a.is_script()) {
if script_path.exists() {
Some(File::open(script_path).await?)
} else {
let needs_script = manifest.package_procedures().any(|a| a.is_script());
let has_script = script_path.exists();
match (needs_script, has_script) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 Neat. I kinda like this pattern, but if we got more than two, maybe having enums to try and prevent boolean blindness?

match (needs_script, has_script) {
  (AreScripts, HasScripts) => ...,
  (AreScript, MissingScripts) => ...,
  (NoScripts, HasScripts) => ...,
  (NoScripts, MissingScripts) => None,

(true, true) => Some(File::open(script_path).await?),
(true, false) => {
return Err(Error::new(eyre!("Script is declared in manifest, but no such script exists at ./scripts/embassy.js"), ErrorKind::Pack).into())
}
} else {
None
(false, true) => {
tracing::warn!("Manifest does not declare any actions that use scripts, but a script exists at ./scripts/embassy.js");
None
}
(false, false) => None
}
})
.build()
Expand Down