don't copy in versionXX_YY imports #93
Conversation
james-bruten-mo
requested review from
cameronbateman-mo,
mo-nikosbaltas,
jennyhickson and
t00sa
and removed request for
a team,
cameronbateman-mo and
mo-nikosbaltas
|
After discussion with Jenny, we've decided we're missing a feature in apply_macros which is required for new metadata sections. I'll "take" this PR back and do that work here. |
|
@jennyhickson and @t00sa I think this is ready to be looked at. Cheers |
| for section in meta_imports: | ||
| section = self.get_full_import_path(section) | ||
| section_missing = [] | ||
| section_macros = [] |
There was a problem hiding this comment.
Rather than having a list of section macros this could just be a len(self.parsed_macros[section] (maybe call it "num_section_macros" to be clearer). If you trigger the after_tag==self.tag condition then just subtract one from the number.
| if after_tag == self.tag: | ||
| continue | ||
| section_macros.append(after_tag) | ||
| if after_tag not in after_tags: | ||
| section_missing.append(after_tag) |
There was a problem hiding this comment.
| if after_tag == self.tag: | |
| continue | |
| section_macros.append(after_tag) | |
| if after_tag not in after_tags: | |
| section_missing.append(after_tag) | |
| if after_tag == self.tag: | |
| num_section_macros -= 1 | |
| elif after_tag not in after_tags: | |
| section_missing.append(after_tag) |
If you go with the above suggestion then this section simplifies
| self.target_macros = {} | ||
| self.parsed_macros = defaultdict(list) |
There was a problem hiding this comment.
A comment here to define target vs parsed macros would be handy
|
|
||
| def combine_missing_macros(self, meta_imports, missing_macros): | ||
| """ | ||
| Combine missing macro commands and write them to the relevant versions.py file. |
There was a problem hiding this comment.
writing them doesn't happen in this routine
| for meta_import in meta_imports: | ||
| for after_tag in missing_macros: | ||
| macro = None | ||
| for m in self.parsed_macros[meta_import]: |
There was a problem hiding this comment.
Some more comments describing the logic in this routine wouldn't go amiss
Description
When a new metadata section with rose app is added, at commit to trunk the app needs to be updated to the latest metadata. This means any imported metadata macros need to be applied to the app and so they need to be copied into the new versions.py by apply_macros. This change adds that functionality.
I've tested this against the new Jules app to ensure it works. I've also checked normal macros work as they currently do. The new functions should get a unit test, but I've not yet added this. I'll open a new issue to add these, and to setup the unit tests to run as CI.
I'm not convinced that the combine_missing_macros function will work perfectly in more complex cases where there are multiple metadata imports. However it is ok for now and will unblock some existing apps tickets.
Checklist