Skip to content

Commit

Permalink
Remove channel from the generate-experimenter command of nimbus-fml
Browse files Browse the repository at this point in the history
  • Loading branch information
jhugman committed Sep 28, 2023
1 parent 277a51c commit 4fcaea4
Show file tree
Hide file tree
Showing 11 changed files with 112 additions and 69 deletions.
2 changes: 1 addition & 1 deletion components/support/nimbus-cli/src/sources/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ impl TryFrom<&ManifestSource> for FeatureManifest {
let files = value.manifest_loader()?;
let path = files.file_path(value.manifest_file())?;
let parser: Parser = Parser::new(files, path)?;
let manifest = parser.get_intermediate_representation(value.channel())?;
let manifest = parser.get_intermediate_representation(Some(value.channel()))?;
manifest.validate_manifest()?;
Ok(manifest)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@ impl From<FeatureManifest> for ManifestFrontEnd {
let enums = merge(&value, |fm| fm.iter_enum_defs().collect(), |e| &e.name);

let about = value.about.description_only();
let channels = value.channel.into_iter().collect();

ManifestFrontEnd {
about: Some(about),
version: "1.0.0".to_string(),
channels: vec![value.channel],
channels,
includes: Default::default(),
imports: Default::default(),
features,
Expand Down
2 changes: 1 addition & 1 deletion components/support/nimbus-fml/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ impl FmlClient {
let files = FileLoader::try_from(&config)?;
let path = files.file_path(&manifest_path)?;
let parser: Parser = Parser::new(files, path)?;
let ir = parser.get_intermediate_representation(&channel)?;
let ir = parser.get_intermediate_representation(Some(&channel))?;
ir.validate_manifest()?;

Ok(FmlClient {
Expand Down
4 changes: 3 additions & 1 deletion components/support/nimbus-fml/src/command_line/cli.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ subcommands:
required: true
index: 2
- channel:
help: The channel to generate the defaults for
# This is no longer needed, but we keep it for backward compatibility.
required: false
help: "Deprecated: The channel to generate the defaults for. This can be omitted."
long: channel
takes_value: true
- cache-dir:
Expand Down
1 change: 0 additions & 1 deletion components/support/nimbus-fml/src/command_line/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ pub(crate) struct GenerateExperimenterManifestCmd {
pub(crate) output: PathBuf,
pub(crate) language: TargetLanguage,
pub(crate) load_from_ir: bool,
pub(crate) channel: String,
pub(crate) loader: LoaderConfig,
}

Expand Down
10 changes: 1 addition & 9 deletions components/support/nimbus-fml/src/command_line/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,17 +106,13 @@ fn create_generate_command_experimenter_from_cli(
let output =
file_path("output", matches, cwd).or_else(|_| file_path("OUTPUT", matches, cwd))?;
let language = output.as_path().try_into()?;
let channel = matches
.value_of("channel")
.map(str::to_string)
.unwrap_or_else(|| RELEASE_CHANNEL.into());
let _channel = matches.value_of("channel").map(str::to_string);
let loader = create_loader(matches, cwd)?;
let cmd = GenerateExperimenterManifestCmd {
manifest,
output,
language,
load_from_ir,
channel,
loader,
};
Ok(cmd)
Expand Down Expand Up @@ -400,7 +396,6 @@ mod cli_tests {
assert!(matches!(cmd, CliCmd::GenerateExperimenter(_)));

if let CliCmd::GenerateExperimenter(cmd) = cmd {
assert_eq!(cmd.channel, "release");
assert_eq!(cmd.language, TargetLanguage::ExperimenterYAML);
assert!(!cmd.load_from_ir);
assert!(cmd.output.ends_with(".experimenter.yaml"));
Expand All @@ -427,7 +422,6 @@ mod cli_tests {
assert!(matches!(cmd, CliCmd::GenerateExperimenter(_)));

if let CliCmd::GenerateExperimenter(cmd) = cmd {
assert_eq!(cmd.channel, "test-channel");
assert_eq!(cmd.language, TargetLanguage::ExperimenterYAML);
assert!(!cmd.load_from_ir);
assert!(cmd.output.ends_with(".experimenter.yaml"));
Expand All @@ -452,7 +446,6 @@ mod cli_tests {
assert!(matches!(cmd, CliCmd::GenerateExperimenter(_)));

if let CliCmd::GenerateExperimenter(cmd) = cmd {
assert_eq!(cmd.channel, "release");
assert_eq!(cmd.language, TargetLanguage::ExperimenterJSON);
assert!(!cmd.load_from_ir);
assert!(cmd.output.ends_with(".experimenter.json"));
Expand Down Expand Up @@ -483,7 +476,6 @@ mod cli_tests {
assert!(matches!(cmd, CliCmd::GenerateExperimenter(_)));

if let CliCmd::GenerateExperimenter(cmd) = cmd {
assert_eq!(cmd.channel, "release");
assert_eq!(cmd.language, TargetLanguage::ExperimenterJSON);
assert!(!cmd.load_from_ir);
assert!(cmd.output.ends_with(".experimenter.json"));
Expand Down
29 changes: 16 additions & 13 deletions components/support/nimbus-fml/src/command_line/workflows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,12 @@ fn generate_struct_single(
manifest_path: FilePath,
cmd: &GenerateStructCmd,
) -> Result<()> {
let ir = load_feature_manifest(files.clone(), manifest_path, cmd.load_from_ir, &cmd.channel)?;
let ir = load_feature_manifest(
files.clone(),
manifest_path,
cmd.load_from_ir,
Some(&cmd.channel),
)?;
generate_struct_from_ir(&ir, cmd)
}

Expand All @@ -101,15 +106,15 @@ fn generate_struct_from_ir(ir: &FeatureManifest, cmd: &GenerateStructCmd) -> Res
pub(crate) fn generate_experimenter_manifest(cmd: &GenerateExperimenterManifestCmd) -> Result<()> {
let files: FileLoader = TryFrom::try_from(&cmd.loader)?;
let path = files.file_path(&cmd.manifest)?;
let ir = load_feature_manifest(files, path, cmd.load_from_ir, &cmd.channel)?;
let ir = load_feature_manifest(files, path, cmd.load_from_ir, None)?;
backends::experimenter_manifest::generate_manifest(ir, cmd)?;
Ok(())
}

pub(crate) fn generate_single_file_manifest(cmd: &GenerateSingleFileManifestCmd) -> Result<()> {
let files: FileLoader = TryFrom::try_from(&cmd.loader)?;
let path = files.file_path(&cmd.manifest)?;
let fm = load_feature_manifest(files, path, false, &cmd.channel)?;
let fm = load_feature_manifest(files, path, false, Some(&cmd.channel))?;
let frontend: ManifestFrontEnd = fm.into();
std::fs::write(&cmd.output, serde_yaml::to_string(&frontend)?)?;
Ok(())
Expand All @@ -119,7 +124,7 @@ fn load_feature_manifest(
files: FileLoader,
path: FilePath,
load_from_ir: bool,
channel: &str,
channel: Option<&str>,
) -> Result<FeatureManifest> {
let ir = if !load_from_ir {
let parser: Parser = Parser::new(files, path)?;
Expand Down Expand Up @@ -190,9 +195,8 @@ pub(crate) fn validate(cmd: &ValidateCmd) -> Result<()> {
))?;
return Ok(());
}
let intermediate_representation = parser
.get_intermediate_representation(&channels[0])
.map_err(|e| {
let intermediate_representation =
parser.get_intermediate_representation(None).map_err(|e| {
output_err(&term, "Manifest is invalid", &e.to_string()).unwrap();
e
})?;
Expand All @@ -218,7 +222,7 @@ pub(crate) fn validate(cmd: &ValidateCmd) -> Result<()> {
let results = channels
.iter()
.map(|c| {
let intermediate_representation = parser.get_intermediate_representation(c);
let intermediate_representation = parser.get_intermediate_representation(Some(c));
match intermediate_representation {
Ok(ir) => (c, ir.validate_manifest()),
Err(e) => (c, Err(e)),
Expand Down Expand Up @@ -305,7 +309,7 @@ mod test {
fn generate_struct_cli_overrides(from_cli: AboutBlock, cmd: &GenerateStructCmd) -> Result<()> {
let files: FileLoader = TryFrom::try_from(&cmd.loader)?;
let path = files.file_path(&cmd.manifest)?;
let mut ir = load_feature_manifest(files, path, cmd.load_from_ir, &cmd.channel)?;
let mut ir = load_feature_manifest(files, path, cmd.load_from_ir, Some(&cmd.channel))?;

// We do a dance here to make sure that we can override class names and package names during tests,
// and while we still have to support setting those options from the commmand line.
Expand Down Expand Up @@ -762,7 +766,7 @@ mod test {
let cmd = create_experimenter_manifest_cmd("fixtures/fe/importing/simple/app.yaml")?;
let files = FileLoader::default()?;
let path = files.file_path(&cmd.manifest)?;
let fm = load_feature_manifest(files, path, cmd.load_from_ir, &cmd.channel)?;
let fm = load_feature_manifest(files, path, cmd.load_from_ir, None)?;
let m: ExperimenterManifest = fm.try_into()?;

assert!(m.contains_key("homescreen"));
Expand Down Expand Up @@ -893,7 +897,6 @@ mod test {
output,
language: TargetLanguage::ExperimenterYAML,
load_from_ir,
channel: "release".into(),
loader,
})
}
Expand All @@ -916,7 +919,7 @@ mod test {
// Load the source file, and get the default_json()
let files: FileLoader = TryFrom::try_from(&loader)?;
let src = files.file_path(&manifest)?;
let fm = load_feature_manifest(files, src, false, channel)?;
let fm = load_feature_manifest(files, src, false, Some(channel))?;
let expected = fm.default_json();

// Generate the merged file
Expand All @@ -931,7 +934,7 @@ mod test {
// Reload the generated file, and get the default_json()
let dest = FilePath::Local(output);
let files: FileLoader = TryFrom::try_from(&loader)?;
let fm = load_feature_manifest(files, dest, false, channel)?;
let fm = load_feature_manifest(files, dest, false, Some(channel))?;
let observed = fm.default_json();

// They should be the same.
Expand Down
Loading

0 comments on commit 4fcaea4

Please sign in to comment.