Fix submission process #701
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…factor-submission-workflow
frankharkins
commented
Apr 30, 2024
Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
* Try to use consistent naming rather than calling everything "sections" * Make the _label_to_id function clearer by just having it transform string to string * Other minor simplifications
frankharkins
commented
May 1, 2024
ecosystem/utils/submission_parser.py
Outdated
| website=website, | ||
| ) | ||
| md_sections = issue_formatted.split("### ")[1:] | ||
| args = dict(_parse_section(s) for s in md_sections) |
There was a problem hiding this comment.
defaultdict isn't needed here as all args have default values
ecosystem/utils/submission_parser.py
Outdated
|
|
||
| def _label_to_id(label: str) -> str: | ||
| """Given a fields "label", find its `id` from the issue template""" | ||
| if not hasattr(_label_to_id, "map"): |
There was a problem hiding this comment.
Imo this is confusing storing the value on the function object itself. Instead, I recommend using a global constant:
LABELs_TO_IDS = _load_labels()
def _load_labels() -> dict[...]:
...Rather than having _label_to_id as a helper function, you can inline the call in parse_section to be LABELs_TO_IDS[label].
--
Edit: another alternative is to define the variable inside parse_submission_issue and then pass it into parse_section. That's probably clearer to avoid global state.
There was a problem hiding this comment.
Thanks! I went with the latter. And I see your point about it getting involved but I'm not sure how to cleanly split into two PRs.
Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
Eric-Arellano
approved these changes
May 1, 2024
Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
The submission process was broken as some entries in the issue template were being ignored by the parser. This PR fixes that and makes the submission process more robust so similar issues don't occur in the future.
Details
Currently, to add a new field to the issue template, you need to edit four different files to make sure the field makes it through to the database. This PR avoids that by explicitly mapping field IDs in the issue template to attributes of the repository model and writing the entry immediately. After this change, you only need to edit the issue template and the repository model.
I've also added a test check the fields in the issue template match fields in the repository model.