George Claghorn

Things you are allowed to do when reviewing a pull request

Think it over. Read the PR completely, understand it as best you can, then put it away. Work on something else for a few hours. Go for a walk. Do laundry. Sleep on it, even. Let it slosh around in your mind. Return to it when your gut has settled.

Don’t hold up your colleague’s work forever. Tell them if you’re going to need more than a few hours. Don’t completely disregard your gut instincts, either. If something changes—you feel differently than you did at first, or something muddy becomes clear—it can be useful to ask why. But don’t be afraid to take your time.

Experiment. Check out the branch locally. Click around.

Try the changes you want to suggest. You may figure out why the author didn’t take the approach you have in mind in the first place, or you may find an even better way.

Poke at the parts of the code that confuse you. They may reveal themselves to you, or you may find a way to clarify them. “Why do we need this?” Rip it out and see what happens. “Does this break if…?” Give it a shot.

Read it many times. Read the changes all the way through once. Draft comments for any questions that pop into your head. When you’re finished, go back and delete the questions that were answered later in the PR.

Writers sometimes change fonts to read their work fresh. Read through the changes in your web browser, then read through them again in your editor of choice. Switch themes, switch fonts, switch between dark and light mode. What stands out?

graphql-rails-namespace

In Namespacing GraphQL constants in Rails apps, I demonstrated how to nest constants defined in a Rails app’s app/graphql/ directory under a top-level ApplicationGraph module with a little bit of configuration.

I extracted the necessary configuration to a tiny Rails plugin and released it as a gem named graphql-rails-namespace. Now all that’s needed, more or less, is bundle add graphql-rails-namespace.

The gem is released under the terms of the MIT License, and the source code is available on GitHub. See the README for more details.

Namespacing GraphQL constants in Rails apps

Update: I’ve extracted the configuration described in this post to a Rails plugin named graphql-rails-namespace. You should use the plugin instead, but I’ll leave this post up for posterity.

The GraphQL Ruby library encourages placing GraphQL modules and classes such as type definitions under the app/graphql/ directory in Rails applications. Here’s an example from the GraphQL Ruby homepage:

# app/graphql/types/profile_type.rb
class Types::ProfileType < Types::BaseObject
  field :id, ID, null: false
  field :name, String, null: false
  field :avatar, Types::PhotoType
end

Because Rails treats all subdirectories of app as root directories for the purpose of autoloading, classes in app/graphql/types/ are nested in a top-level Types module. In the above example, app/graphql/types/profile_type.rb defines Types::ProfileType. But Types is too general a top-level namespace for GraphQL type definitions. The GraphQL schema class defined in app/graphql/schema.rb is named Schema, but it’s specifically a GraphQL schema, not a generic schema. It would be nice to nest GraphQL-specific modules and classes in a GraphQL-specific namespace.

Keep reading…

Hotwire: HTML over the wire

New from some of my colleagues:

Hotwire is an alternative approach to building modern web applications without using much JavaScript by sending HTML instead of JSON over the wire. This makes for fast first-load pages, keeps template rendering on the server, and allows for a simpler, more productive development experience in any programming language, without sacrificing any of the speed or responsiveness associated with a traditional single-page application.

It’s been exciting to watch Hotwire come together at work, and it’s made the HEY frontend development experience a real treat.

The Basecamp security bug bounty

I shared what I’ve been working on lately on Signal v. Noise:

The Basecamp security bug bounty program is now open to the public on HackerOne. Our security team is ready to take vulnerability reports for Basecamp 3 and HEY. Bounties range from $100 to $10,000. We pay more for more severe vulnerabilities, more creative exploits, and more insightful reports.

Discovering the D/B flag

When a Multiboot 2-compliant boot loader like GRUB invokes a 32-bit x86 operating system, it provides a magic number in the EAX register (0x36d76289) and the physical address of a boot information table in EBX.

These values will eventually be useful to Georgix, so the first thing its bootstrap code does is push them onto the stack (in reverse order, so they can be popped off in order):

_start:
    # Set up the stack.
    movl $boot.stack.high, %esp

    # The bootloader provides:
    #
    # * A magic number in EAX
    # * The physical address of the boot information record in EBX
    #
    # Save these values on the stack to pass to the Rust entrypoint later.
    pushl %ebx
    pushl %eax

After a bit of setup, the bootstrap code jumps to the Rust entrypoint, passing the magic number and boot information table pointer as arguments. Per the C calling convention, the first two arguments are passed in the EDI and ESI registers:

popl %edi
popl %esi
ljmp $boot.global_descriptor_table.code, $main

The bootstrap code didn’t always work this way. Until recently, it saved the magic number and boot information table pointer in EDI and ESI rather than on the stack. I wanted to use the stack from the start—to clarify that these values aren’t needed for setup, and to free up EDI and ESI for other purposes—but ran into some trouble.

Keep reading…

Switching from the legacy 8259 PIC to the modern APIC

The Writing an OS in Rust series covers enabling periodic timer interrupts with the 8259 PIC. As it explains, the 8259 is superseded by the APIC in modern x86-64 processors:

The Intel 8259 is a programmable interrupt controller (PIC) introduced in 1976. It has long been replaced by the newer APIC, but its interface is still supported on current systems for backwards compatibility reasons. The 8259 PIC is significantly easier to set up than the APIC, so we will use it to introduce ourselves to interrupts before we switch to the APIC in a later post.

Once I got timer interrupts working with the 8259 per the tutorial, I read up on using the APIC—particularly Xv6’s LAPIC initialization code—and decided to make the switch right away.

Keep reading…

Writing an operating system

If you follow me on Twitter, you may remember that I’m writing an x86-64 operating system in Rust to learn about operating systems. I learn best by doing and by writing. I’m sharing my work on GitHub. I’ll use this blog to share my writing.

So far, Georgix—that’s my operating system’s name, because I’m not creative enough to come up with something less obnoxious—boots, configures periodic timer interrupts, and handles them by printing dots to the screen. That’s not much, of course, but I’ve already learned a ton and I’m looking forward to learning more. Next up is memory allocation and paging.

Keep reading…