Miscellaneous

Discussion of all aspects of the game engine, including development of new and existing features.

Moderator: Forum Moderators

Post Reply
CrawlCycle
Posts: 27
Joined: November 20th, 2020, 5:07 am

Miscellaneous

Post by CrawlCycle »

Commenting a workaround for an external dependency

There are 13 usages of the noun "workaround" and 8 usages of the verb "work around" in the .cpp and .hpp files.
game.hpp

Code: Select all

/**
 * Default constructor, defined out of line to work around a warning in
 * gcc 4.5.2
 */
surface.cpp

Code: Select all

/* Workaround for an SDL bug.
 * SDL 2.0.6 frees the blit map unconditionally in SDL_FreeSurface() without checking
 * if the reference count has fallen to zero. However, many SDL functions such as
 * SDL_ConvertSurface() assume that the blit map is present.
 * Thus, we only call SDL_FreeSurface() if this is the last reference to the surface.
 * Otherwise we just decrement the reference count ourselves.
 *
 * - Jyrki, 2017-09-23
 */
I want a format that would:
  • allow easy search for workarounds for a library in the code base
  • describe the versions of the external library that require the workaround
  • mention that the external bug has been fixed in version X.Y of the external library
I am thinking about this format: "Work around component<version bug [fixed in version]".

Code: Select all

// Work around a Boost<1.67 bug fixed in 1.67: Include test_case.hpp after
// unit_tests.hpp. https://svn.boost.org/trac10/ticket/13387
Last edited by CrawlCycle on November 28th, 2020, 1:17 am, edited 6 times in total.
User avatar
Pentarctagon
Project Manager
Posts: 4735
Joined: March 22nd, 2009, 10:50 pm
Location: Earth (occasionally)

Re: Miscellaneous

Post by Pentarctagon »

Keep in mind that at the time the comment was written, the bug might not have been fixed in any version yet.
99 little bugs in the code, 99 little bugs
take one down, patch it around
-2,147,483,648 little bugs in the code
CrawlCycle
Posts: 27
Joined: November 20th, 2020, 5:07 am

Re: Miscellaneous

Post by CrawlCycle »

Workaround for Boost>=1.67 bug would represent a bug that has not been fixed since version 1.67.

Is the grammar of the comment correct?
I think it should have an extra "a": Workaround for a Boost>=1.67 bug.

But sometimes people might forget to add the "a". In that case, searching for "Workaround for a Boost" would not actually find all workarounds for boost.
User avatar
Pentarctagon
Project Manager
Posts: 4735
Joined: March 22nd, 2009, 10:50 pm
Location: Earth (occasionally)

Re: Miscellaneous

Post by Pentarctagon »

That seems fine to me.
99 little bugs in the code, 99 little bugs
take one down, patch it around
-2,147,483,648 little bugs in the code
CrawlCycle
Posts: 27
Joined: November 20th, 2020, 5:07 am

Re: Miscellaneous

Post by CrawlCycle »

I think I will settle with this one:

Code: Select all

// Work around a Boost<1.67 bug fixed in 1.67: Include test_case.hpp after
// unit_tests.hpp. https://svn.boost.org/trac10/ticket/13387
User avatar
Celtic_Minstrel
Developer
Posts: 1798
Joined: August 3rd, 2012, 11:26 pm
Location: Canada
Contact:

Re: Miscellaneous

Post by Celtic_Minstrel »

I don't know if it's possible in all cases to decide whether or not to put a space in "workaround". My impression is that if used as a verb, it should have a space, but when used as a noun, it should not. So if you intend to change all instances to be exactly the same, you probably need to make sure to phrase it as a noun (because if phrasing as a verb it could be either "work around" or "works around").

I'm not sure this is even a good idea, though - if all you want is for it to be searchable, then can't you use some sort of fixed tag an addition to the natural language explanation? Something like a TODO: tag but probably distinct.
CrawlCycle wrote: November 25th, 2020, 7:08 pm Workaround for Boost>=1.67 bug would represent a bug that has not been fixed since version 1.67.

Is the grammar of the comment correct?
I think it should have an extra "a": Workaround for a Boost>=1.67 bug.

But sometimes people might forget to add the "a". In that case, searching for "Workaround for a Boost" would not actually find all workarounds for boost.
It's grammatically correct both with and without the "a". However, leaving out the "a" might be equivalent to adding "the" instead. I'm not entirely sure; it can be ambiguous when there's no article whether it's definite or indefinite.

In this case, I don't think it makes any difference whether it's definite or indefinite. "a Boost bug" and "the Boost bug" both get the correct meaning across, so just "Boost bug" is equally fine. It's "a bug in Boost" and it's also "the bug in Boost that caused this".
Author of The Black Cross of Aleron campaign and Default++ era.
Maintainer of Steelhive.
User avatar
GunChleoc
Translator
Posts: 504
Joined: September 28th, 2012, 7:35 am
Contact:

Re: Miscellaneous

Post by GunChleoc »

How about:

Code: Select all

Workaround: Boost>=1.67 bug
It's less natural languagey but also easier to deal with for non-native speakers of English.
Post Reply