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

Replace code block-based issue template with GitHub issue form #558

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

jfy133
Copy link
Collaborator

@jfy133 jfy133 commented May 24, 2023

I think this might be a bit easier for users to rapidly make the term request, as they can copy and paste into clear blocks, without having to remove the placeholder texts.

I'm happy to convert the other templates as well if you are interested.

@turbomam turbomam self-requested a review October 18, 2023 00:01
Copy link
Member

@turbomam turbomam left a comment

Choose a reason for hiding this comment

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

Awesome. Thank you. Apologies for the slow reply.

Since MIxS 6.2+ is LinkML based, there isn't any such thing as a "Structured comment name" anymore (etc., etc.). I can help you align the terminology in this template with the new vocabulary of term attributes if you want.

@jfy133
Copy link
Collaborator Author

jfy133 commented Oct 18, 2023

Hi @turbomam - sure!

I'm still on parental leave and today I have to travel, so it will be a bit of time until I could get to this.

Feel free to edit my fork directly and merge if you have permissions, otherwise you can make code suggestions on the PR and I can commit them (something j can do at least from the GitHub app)

@jfy133
Copy link
Collaborator Author

jfy133 commented Oct 21, 2023

@turbomam I've updated based on what @sujaypatil96 linked to in #678. I've moved the 'old names' into the description, and added two new questions for the unit/multivalue entries.

@turbomam
Copy link
Member

turbomam commented Nov 6, 2023

@sujaypatil96 please integrate this into one of your forks so that we can go through the experience of creating an issue based on this new YAML template

@sujaypatil96
Copy link
Collaborator

@jfy133 it looks like GitHub is complaining about some YAML syntax error in your issue form: https://github.com/jfy133/genomics-standards-consortium-mixs/blob/main/.github/ISSUE_TEMPLATE/TERM-REQUEST.yml

Could you fix that?

@jfy133
Copy link
Collaborator Author

jfy133 commented Nov 7, 2023

Done, sorry about that - uninformative errors took a bit of experimenting.

Please use the GitHub 'view file' functionality to make sure you're happy with everything (e.g. I can't remember if Multivalued was meant to be required or not) etc.

@only1chunts
Copy link
Member

Since MIxS 6.2+ is LinkML based, there isn't any such thing as a "Structured comment name" anymore (etc., etc.). I can help you align the terminology in this template with the new vocabulary of term attributes if you want.

Hi @turbomam , I agree that structured comment name is not a LinkML thing, but it is still what we call it in GSC-MIXS, so I think it best that it is shown as such in the template.
@jfy133, I love this new template, thanks for creating it! Please can you revert the "Title" to "Structured Comment Name", thanks.

@jfy133
Copy link
Collaborator Author

jfy133 commented Nov 7, 2023

@only1chunts it's still listed as in the 'description', or would you rather still have it as the 'primary' name?

But otherwise: @turbomam @sujaypatil96 you're more than welcome to directly add the PR either with comment suggestion or push to branch

@only1chunts
Copy link
Member

@only1chunts it's still listed as in the 'description', or would you rather still have it as the 'primary' name?
Sorry, I'm not sure I understand your question, but hopefully this will clarify what I meant:
image

@jfy133
Copy link
Collaborator Author

jfy133 commented Nov 7, 2023

To clarify what I mean

280994996-9972f670-a3b3-4300-b799-3e1a28d74652

@only1chunts
Copy link
Member

OK, So that "previously known as" bit should be removed as well then (its not "previously known as" it is still known as), if you want to add what the appropriate LinkML word is for it there instead then that could be done (I guess in LinkML its called "title" is it? which sounds rather mis-leading to me as its not a title at all, its really just an alternative name that was created entirely for the INSDC because they auto truncate names longer than 20char)

@jfy133
Copy link
Collaborator Author

jfy133 commented Nov 7, 2023

Ah ok fair enough.

I was just following the request above to follow the table from the Tech group meeting back in June.

I think it's best if you and, Sujay, Mark discuss it and edit the PR (as it's just terminology now I guess), as I have no opinion either way, and then merge when you have a consensus :)

@turbomam
Copy link
Member

turbomam commented Nov 7, 2023

Thanks for the great discussion guys

@only1chunts what is the requirement for continuing to use language like "Structured comment name"? Or heavens forbid "Expected value"? Those terms aren't defined anywhere in the schema. I don't think they are even defined anywhere in the whole MIxS repo. Where are they defined in some actionable way?

This repo will be harder for everyone to understand if we use terminology that isn't tightly coupled with the repo artifacts. If someone wants to look through the schema to see examples of existing "Structured comment names", how would they do that?

We had an agreement about mappings between the old nomenclature and the new LinkML-biased nomenclature, although I will admit that we didn't agree to retire the legacy MIxS nomenclature.

I think a form with a header called name and a hint "previously called Structured comment name" is the ideal bridge between theses two nomenclature systems.

@cmungall has made a very preliminary proposal for mapping between LinkML metaslots (like name, title, etc) and project-specific metaslots (like "Structured comment name") but there is no implementation yet. If it is truly a requirement to keep all of the legacy, implicit MIxS metaslots in the model itself, then GSC should sponsor/support the work that would be required to implement this.

@only1chunts
Copy link
Member

the issue here was that it was proposed that "structured comment name" would be called "Title", which is not something that has been proposed before, but as you suggest and as the document you point to suggests, changing "structured comment name" to "name" is acceptable and should not confuse anyone. But it does mean the template layout needs to be corrected in the wording to ensure consistent use of the mapped terminology.

@only1chunts
Copy link
Member

I dont know how to go about suggesting changes to the file here, so as its just one small file I've copied and pasted it here with my suggested changes:

name: New Term Request
description: "For us to assess a new term request we require the following details:"
title: "New term proposal: "
labels: [NewTerm]
body:
  - type: checkboxes
    attributes:
      label: Checklist
      description: >
        Please make sure you check all these items before submitting your feature request.
      options:
        - label: I have reviewed the existing MIxS checklists and extensions that the term does not exist.
          required: true
  - type: input
    attributes:
      label: Title
      description: >
        The name of the term, previousy known as Package item name or Label
    validations:
      required: true
  - type: input
    attributes:
      label: Name
      description: >
        A version of the name with less than 20 characters and no spaces. Previously known as Structured comment name.
    validations:
      required: true
  - type: textarea
    attributes:
      label: Description
      description: >
        A clear and concise definition of the term
    validations:
      required: true
  - type: input
    attributes:
      label: Expected value
      description: >
        A short description of the expected pattern or value range of information the term will hold, e.g. text or EFO and/or OBI
      placeholder: >
        "e.g., text or EFO and/or OBI etc...".
    validations:
      required: true
  - type: input
    attributes:
      label: Value syntax
      description: >
        The pattern of the value that the term will be checked against
      placeholder: >
        "e.g. {float} {unit}|{termLabel} {[termID]}|{text}|{timestamp} etc...".
    validations:
      required: true
  - type: input
    attributes:
      label: Example
      description: >
        Please provide a valid example value.
    validations:
      required: true
  - type: input
    attributes:
      label: Preferred unit
      description: >
        Indicate the preferred unit of measurement if appropriate
      placeholder: >
        "e.g, millliter, gram, milligram, liter".
    validations:
      required: false
  - type: input
    attributes:
      label: Extensions(s)
      description: >
        List any extensions that should include the new term
    validations:
      required: false

@jfy133
Copy link
Collaborator Author

jfy133 commented Nov 7, 2023

From my phone so can't send screenshot but to make suggestion detailed instructions are as follows

  • go to files tab on this PR
  • scroll to the line you want to change
  • hover over the line to the left hand side of the file pane (near or on the line number itself
  • a blue plus button should appear, press it
  • this will open a comment box
  • in the comment box, in the editor tool bar, there should be a symbol like a +- stacked on top of each other, press that
  • a markdown code block should appear with the contents of the line
  • edit the code block to how you want the line to be
  • once happy, press the 'add single comment' button, or if you make Multiple line edits, press the green 'start a review'

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.

None yet

4 participants