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

Use create() instead of new() when processing templates #30

Merged
merged 13 commits into from
Feb 6, 2024

Conversation

lhh
Copy link
Owner

@lhh lhh commented Jan 26, 2024

With this change, you can use any custom field you could use from the CLI within templates using the nym or field name. Ex: story_points, or fixversions

@lhh lhh force-pushed the schema_use_create branch 2 times, most recently from 522c0b6 to c63f656 Compare January 26, 2024 15:27
@lhh lhh requested a review from compi-migui January 26, 2024 17:22
jirate/jira_cli.py Outdated Show resolved Hide resolved
jirate/jira_cli.py Outdated Show resolved Hide resolved
jirate/jira_cli.py Outdated Show resolved Hide resolved
jirate/jira_cli.py Outdated Show resolved Hide resolved
jirate/jira_cli.py Outdated Show resolved Hide resolved
@lhh lhh force-pushed the schema_use_create branch 2 times, most recently from edacace to c64fb1a Compare February 2, 2024 15:51
lhh added 11 commits February 2, 2024 13:58
Fields in simple fields (fields we never modify) were not being
pasted to output dict, making it difficult to write tests
Does not include multiple projects since our test data
doesn't have multiple projects and isn't written that
way at the moment.

The pycontribs/jira project utilizes a test environment
which spawns an instance of Jira Server live during CI.
We may want to consider pivoting how we do CI to that
model so we don't keep having to fabricate what Jira
would do.
compi-migui
compi-migui previously approved these changes Feb 5, 2024
Copy link
Collaborator

@compi-migui compi-migui left a comment

Choose a reason for hiding this comment

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

Just a couple very much non-blocking things, I would merge as-is unless you feel strongly about either of those.

def issue_metadata(self, issue_type_or_id):
def issue_metadata(self, issue_type_or_id, project_key=None):
if not project_key:
project_key = self.project_name
itype = None
for issuetype in self.issue_types:
Copy link
Collaborator

Choose a reason for hiding this comment

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

issue (non-blocking): We're iterating through the default project's issuetypes even when getting metadata from a different project. This only works as expected if the issuetype in question is in the intersection of both projects, and names/IDs match for both.
suggestion: I doubt it'll matter in most situations and we can just leave it as an edge case to be filled in once someone actually runs into it. It seems rather obscure and not worth holding up the PR.

reserved_fields = ['subtasks', 'issuetype', 'parent']
# required_fields are the same
start_fields = {'issuetype': _subtask, 'project': pname}
start_fields['parent'] = parent.key
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit (non-blocking): we can set the parent as we initialize the dictionary, like:
start_fields = {'issuetype': _subtask, 'project': pname, 'parent': parent.key}

Copy link
Owner Author

Choose a reason for hiding this comment

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

Let's not block for this for now.

@compi-migui
Copy link
Collaborator

suggestion (blocking): Just considered we should also change the issue schema and the example/test templates to use issuetype instead of issue_type:
https://github.com/lhh/jirate/blob/master/jirate/schemas/issue.yaml#L17

https://github.com/lhh/jirate/blob/master/contrib/example-template.yaml#L2
https://github.com/lhh/jirate/blob/master/jirate/tests/templates/good-template.yaml#L2
https://github.com/lhh/jirate/blob/master/jirate/tests/templates/bad-template.yaml#L2

Otherwise we're stuck in a place where the schema validation wants issue_type but the issue filing code wants issuetype and templates can't make both of them happy without duplicating the field.

A conversation for another day is whether having a schema at all still makes sense now that the code looks at required/reserved fields and all. But for now the quick fix will let us merge/tag/release and move on.

Copy link
Collaborator

@compi-migui compi-migui left a comment

Choose a reason for hiding this comment

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

@lhh
Copy link
Owner Author

lhh commented Feb 5, 2024

suggestion (blocking): Just considered we should also change the issue schema and the example/test templates to use issuetype instead of issue_type: https://github.com/lhh/jirate/blob/master/jirate/schemas/issue.yaml#L17

https://github.com/lhh/jirate/blob/master/contrib/example-template.yaml#L2 https://github.com/lhh/jirate/blob/master/jirate/tests/templates/good-template.yaml#L2 https://github.com/lhh/jirate/blob/master/jirate/tests/templates/bad-template.yaml#L2

Otherwise we're stuck in a place where the schema validation wants issue_type but the issue filing code wants issuetype and templates can't make both of them happy without duplicating the field.

A conversation for another day is whether having a schema at all still makes sense now that the code looks at required/reserved fields and all. But for now the quick fix will let us merge/tag/release and move on.

Done.

@compi-migui compi-migui merged commit fe4edd0 into master Feb 6, 2024
3 checks passed
@lhh lhh deleted the schema_use_create branch February 14, 2024 12:15
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.

2 participants