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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

My first exercise in open source bug fixing. "Limit extra long wiki titles" #1

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@Sleepsounder
Copy link

commented Apr 10, 2019

Fixes publiclab#5426

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • PR is descriptively titled 馃搼 and links the original issue above 馃敆
  • tests pass -- look for a green checkbox 鉁旓笍 a few minutes after opening your PR -- or run tests locally with rake test
  • code is in uniquely-named feature branch and has no merge conflicts 馃搧
  • screenshots/GIFs are attached 馃搸 in case of UI updation
  • ask @publiclab/reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

@Thrillberg

This comment has been minimized.

Copy link

commented Apr 11, 2019

Hey Geoff, Great job on your first PR! From what I understand, this satisfies the issue. A couple things come to mind, however:

  1. You should go ahead and check the checkboxes in the comment above to indicate completion of those 5 tasks, as appropriate.
  2. I'm not sure why tests didn't run (maybe because we forked the repo or something?) but that would be something to perhaps get in touch with the maintainers about.
  3. They recommend screenshots/gifs for issues, where appropriate. I think this is a great idea (especially since the original issue had a gif, right?) and is a good exercise. Basically, you want to make a screenshot to show what it looks like after your fix. Do you know how to take a screenshot? I use Skitch, personally, but Mac has a built-in command (Command + Shift + 4) and then click and drag a box around the area you want to capture in an image. You'd then comment with a screenshot here to show others.
  4. Lastly, when all is in good shape, you can ask the maintainers to take a look at this, as indicated in the comment above.

Again, great job and looking forward to seeing this merged in, hopefully! 馃帀

@olbrich

This comment has been minimized.

Copy link

commented Apr 11, 2019

@Thrillberg The travis configs are setup to only run on a couple of branches, and this isn't one of them.

@olbrich
Copy link

left a comment

馃憤 Please add a screenshot 馃摲!

@Sleepsounder

This comment has been minimized.

Copy link
Author

commented Apr 11, 2019

Thanks for your feedback! I've attached two screenshots. One screenshot is with the amended code in the html file (line 61 - maxlength="50"). The other screenshot demonstrates before the change (very long "lorem ipsum" title) and after the change (cut-off 50 character "lorem ipsum" title). Is this the right idea for what the screenshots should demonstrate?

Screen Shot 2019-04-11 at 11 17 32 AM

Screen Shot 2019-04-11 at 11 16 12 AM

@Sleepsounder

This comment has been minimized.

Copy link
Author

commented Apr 11, 2019

I also did a rake test and couldn't see anywhere if my little change was accounted for. Maybe I'll heed your advice, Eric, and message them.

@Sleepsounder

This comment has been minimized.

Copy link
Author

commented Apr 11, 2019

It looks like Jeremy snagged this fix in the the original public lab repository - which is cool. I'll ask him about the testing and what he found. This has been a good exercise for me to start. Feedback is welcome and appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.