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

Adds yard and makefile with development commands #50

Merged
merged 1 commit into from
Jun 27, 2024
Merged

Conversation

JPrevost
Copy link
Member

  • adds makefile with useful development commands
  • adds yard gem and documentation on how to run the local server

yard is not perfect, but the value add is enough to include it at this time while we consider if this is a tool we want to use (either as-is or with modified templates)

https://mitlibraries.atlassian.net/browse/TCO-36

Copy link
Contributor

@jazairi jazairi left a comment

Choose a reason for hiding this comment

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

Confirmed that all make commands work as expected. I suggested an update to the reference docs, as it seems that make docserver will not work unless you first install the gem.

Poked around the generated docs a bit, as well, but will do more once #49 lands. I think this will make a big improvement to our documentation practices; thanks for suggesting and implementing it!

@@ -26,4 +34,6 @@

### Reference

`make docserver` will start a `yard` server using the RDoc comments from the codebase. RDoc in this application is a work-in-progress and should improve over time. As of this writing, the index page generated contains broken links to our markdown documentation, but they "files" navigation displays them properly.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest we document the prerequisite of gem install yard here. Without that, the make command fails (for me, anyway).

Copy link
Member Author

Choose a reason for hiding this comment

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

bundle install should cover that as it's in the bundle right?

Copy link
Member Author

Choose a reason for hiding this comment

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

We could have make install be run prior to every run of make docserver but I thin that would get annoying.

I'll update the readme to know that prior to running for the first time, bundle install or make install needs to be run.

My initial reaction was "but that's always the case for gems", but after having sat with this for a bit I think it's worth noting explicitly for this as it likely isn't clear it's a bundled gem that is doing this work. Thanks for pointing it out.

Copy link
Contributor

Choose a reason for hiding this comment

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

I could've sworn that I got that error after running bundle install, which is why I mentioned it. I just tried this, though:

  1. Run gem uninstall yard.
  2. Try make docserver: returns rbenv: yard: command not found, as expected.
  3. Run bundle install.
  4. Try make docserver: the yard server starts up successfully.

I was so baffled by this that I looked at my shell history from where I first checked out the branch:

10123  git pull
10124  git checkout yard-and-makefile
10125  bundle install
10126  make docserver (this failed with 'command not found')
10127  rm -rf .yardoc (tried clearing the cache as suggested, in case there was any)
10128  make docserver (failed again)
10129  gem install yard
10130  yard server --reload (working now)

After that, make docserver worked as well, as expected. So, I think it's still possible for yard not to work even if you do run bundle install first, but I'm not exactly sure why. Since I can't replicate it, I think I'm fine with the additional tip text as-is.

Copy link
Member Author

Choose a reason for hiding this comment

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

Super weird. Thanks for confirming so we can keep an eye out for this in the future.

@JPrevost JPrevost temporarily deployed to tacos-api-pipeline-pr-50 June 27, 2024 18:30 Inactive
@JPrevost JPrevost requested a review from jazairi June 27, 2024 18:30
Copy link
Contributor

@jazairi jazairi left a comment

Choose a reason for hiding this comment

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

See my non-blocking comment. Something very weird was happening in my shell, but I can't replicate it. I think we should have Matt try the build process when he returns to see if that was a one-off oddity or something that should be documented.

@@ -26,4 +34,6 @@

### Reference

`make docserver` will start a `yard` server using the RDoc comments from the codebase. RDoc in this application is a work-in-progress and should improve over time. As of this writing, the index page generated contains broken links to our markdown documentation, but they "files" navigation displays them properly.
Copy link
Contributor

Choose a reason for hiding this comment

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

I could've sworn that I got that error after running bundle install, which is why I mentioned it. I just tried this, though:

  1. Run gem uninstall yard.
  2. Try make docserver: returns rbenv: yard: command not found, as expected.
  3. Run bundle install.
  4. Try make docserver: the yard server starts up successfully.

I was so baffled by this that I looked at my shell history from where I first checked out the branch:

10123  git pull
10124  git checkout yard-and-makefile
10125  bundle install
10126  make docserver (this failed with 'command not found')
10127  rm -rf .yardoc (tried clearing the cache as suggested, in case there was any)
10128  make docserver (failed again)
10129  gem install yard
10130  yard server --reload (working now)

After that, make docserver worked as well, as expected. So, I think it's still possible for yard not to work even if you do run bundle install first, but I'm not exactly sure why. Since I can't replicate it, I think I'm fine with the additional tip text as-is.

@jazairi jazairi self-assigned this Jun 27, 2024
- adds makefile with useful development commands
- adds `yard` gem and documentation on how to run the local server

`yard` is not perfect, but the value add is enough to include it at this time while we consider
if this is a tool we want to use (either as-is or with modified templates)

https://mitlibraries.atlassian.net/browse/TCO-36
@JPrevost JPrevost force-pushed the yard-and-makefile branch from 68f6de2 to 9cc4f95 Compare June 27, 2024 21:16
@JPrevost JPrevost temporarily deployed to tacos-api-pipeline-pr-50 June 27, 2024 21:17 Inactive
@JPrevost JPrevost merged commit 17c3abc into main Jun 27, 2024
1 check passed
@JPrevost JPrevost deleted the yard-and-makefile branch June 27, 2024 21:17
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.

3 participants