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

Update runners.html #12883

Closed
wants to merge 1 commit into from
Closed

Conversation

gmjoloch
Copy link
Contributor

@gmjoloch gmjoloch commented May 9, 2024

Submission Checklist

Required

  • The pull request title clearly contains the name of the sheet I am editing.
  • The pull request title clearly states the type of change I am submitting (New Sheet/New Feature/Bugfix/etc.).
  • The pull request makes changes to files in only one sub-folder.
  • The pull request does not contain changes to any json files in the translations folder (translation.json is permitted)

New Sheet Details

  • The name of this game is: < > (i.e. Dungeons & Dragons 5th Edition, The Dresden Files RPG)

  • The publisher of this game is: < > (i.e. Wizards of the Coast, Evil Hat)

  • The name of this game system/family is: < > (i.e. Dungeons & Dragons, FATE)

  • I have followed the Character Sheets Standards when building this sheet.

  • I have authorization from the game's publisher to make this an official sheet on Roll20 with their name attached.
  • This game is not a traditionally published game, but a copy of the game rules can be purchased/downloaded/found at: < >
  • This sheet is for an unofficial fan game, modification to an existing game, or a homebrew system.

Changes / Description

@roll20deploy
Copy link
Contributor

Character Sheet Info Roll20 Internal Use only.

@BronsonHall
Copy link
Contributor

Hey @gmjoloch ,

Couple small requests here before we can merge this:

  1. It helps us quite a bit if you add a summary to your PR describing the changes you're making for the sheet, even if it's a very small change. I notice you've also got separate PRs for the html update and css update. As they appear connected, you'll want to create a branch, make both file changes on the same branch, and then submit a single PR with both updated files. That way one doesn't get merged without the other.
  2. You're changing a lot of attr_ names here. That can be an issue for existing sheets, which already have data stored for those attributes. If the name on the sheet changes, they'll lose that data, and have to input it again. If it's not too much trouble, we generally ask that attribute name changes come with sheet worker helpers to move old input data into the new.

I'm going to block this PR for now, and close the css one. Let me know if I've misinterpreted anything here or if you have any questions!

@BronsonHall BronsonHall mentioned this pull request May 14, 2024
8 tasks
@gmjoloch gmjoloch closed this May 17, 2024
@gmjoloch gmjoloch deleted the gmjoloch-patch-1 branch May 17, 2024 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants