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

Checklist: styxdocker #14

Closed
1 task done
shnizzedy opened this issue Nov 18, 2024 · 4 comments
Closed
1 task done

Checklist: styxdocker #14

shnizzedy opened this issue Nov 18, 2024 · 4 comments
Labels
approved Approval for tool checklist checklist Denote checklist submission

Comments

@shnizzedy
Copy link
Member

shnizzedy commented Nov 18, 2024

Checklist content

{
  "name": "styxdocker",
  "urls": [
    {
      "url_type": "Documentation",
      "url": "https://childmindresearch.github.io/styxdocker"
    },
    {
      "url_type": "Source code",
      "url": "https://github.com/childmindresearch/styxdocker"
    }
  ],
  "documentation": {
    "bronze": {
      "1": true,
      "2": true,
      "3": true,
      "4": true,
      "5": true,
      "6": true,
      "7": true,
      "8": true,
      "9": true
    },
    "silver": {
      "1": true,
      "2": false,
      "3": false,
      "4": true,
      "5": true,
      "6": true
    },
    "gold": {
      "1": false,
      "2": true,
      "3": true,
      "4": true,
      "5": false,
      "6": true
    }
  },
  "infrastructure": {
    "bronze": {
      "1": true,
      "2": true,
      "3": true,
      "4": true,
      "5": true,
      "6": false,
      "7": true
    },
    "silver": {
      "1": true,
      "2": true,
      "3": true
    },
    "gold": {
      "1": true,
      "2": true,
      "3": false,
      "4": true,
      "5": true
    }
  },
  "testing": {
    "bronze": {
      "1": false,
      "2": false
    },
    "silver": {
      "1": true,
      "2": false
    },
    "gold": {
      "1": false,
      "2": false
    }
  }
}

Additional information

  • Documentation is up to date with version of software

    • Documentation is built as part of CI, but does not document version in output
  • Typical intended usage is described

    • “Now you can use any Styx functions as usual, and they will run in Docker containers” assumes we’re familiar with Styx functions; doesn’t link to Styx documentation.
  • Background/significance of program

    • “offering improved isolation and reproducibility for your workflows.” Improved from what?
  • Thorough description of required and optional input parameters

    • “data_dir: Directory for temporary data storage” Local or in container?
  • Continuous integration badges in README for build status

    • Implied by tests passing?

Agreement

  • I agree
@shnizzedy shnizzedy added the checklist Denote checklist submission label Nov 18, 2024
@LuciMoore
Copy link
Collaborator

final review was submitted here: #35

moving forward, we'll try to avoid creating new issues and instead update the original issues

@kaitj tagging you for awareness!

@LuciMoore LuciMoore reopened this Nov 27, 2024
@LuciMoore
Copy link
Collaborator

The final review for this has been performed. Additional notes from the final reviewer:

  • Only has one source code file. Not sure that this is an actual Python project. I guess it’s a Docker thing.
  • There aren’t any real tests in this. There is test_dummy.py which is maybe pro forma. Not sure if that matters since this is a Docker project. I know very little about Docker.
  • No real style guide, although there is a CONTRIBUTING.md file.
  • There are really no tests for continuous integration to run.
  • There is a testing suite present but it’s not really used.

I've copied the original checklist json below and will update the json above with the checklist from the final review before marking as approved

{
  "name": "styxdocker",
  "urls": [
    {
      "url_type": "Documentation",
      "url": "https://childmindresearch.github.io/styxdocker"
    },
    {
      "url_type": "Source code",
      "url": "https://github.com/childmindresearch/styxdocker"
    }
  ],
  "documentation": {
    "bronze": {
      "1": true,
      "2": true,
      "3": true,
      "4": true,
      "5": true,
      "6": true,
      "7": true,
      "8": true,
      "9": true
    },
    "silver": {
      "1": true,
      "2": false,
      "3": false,
      "4": true,
      "5": true,
      "6": true
    },
    "gold": {
      "1": false,
      "2": true,
      "3": true,
      "4": true,
      "5": true,
      "6": true
    }
  },
  "infrastructure": {
    "bronze": {
      "1": true,
      "2": true,
      "3": true,
      "4": true,
      "5": true,
      "6": false,
      "7": true
    },
    "silver": {
      "1": true,
      "2": true,
      "3": true
    },
    "gold": {
      "1": true,
      "2": true,
      "3": false,
      "4": true,
      "5": true
    }
  },
  "testing": {
    "bronze": {
      "1": false,
      "2": false
    },
    "silver": {
      "1": true,
      "2": false
    },
    "gold": {
      "1": false,
      "2": false
    }
  }
}

@LuciMoore LuciMoore added the approved Approval for tool checklist label Nov 27, 2024
github-actions bot added a commit that referenced this issue Nov 27, 2024
@kaitj
Copy link
Collaborator

kaitj commented Dec 2, 2024

Closing this manually, bot should have automatically closed it from message.

Also ccing @nx10 for general awareness of reviewer comments

@kaitj kaitj closed this as completed Dec 2, 2024
@shnizzedy
Copy link
Member Author

Only has one source code file. Not sure that this is an actual Python project. I guess it’s a Docker thing.

It's a real Python project, packaged for and published on PyPI. I think it's just one script because it's adhering to Unix philosophy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approval for tool checklist checklist Denote checklist submission
Projects
None yet
Development

No branches or pull requests

3 participants