Review of attachment 345166 [details]:

Dispite a few things to fix, it globally looks good.

::: plugins/nes/src/nes-plugin.vala
@@ +3,2 @@
 private class Games.NesPlugin : Object, Plugin {
+	private const string FINGERPRINT_NES_PREFIX = "nes";

Good point to use NES to distinguish the prefixes, though it would be better to put it before (like "NES_FINGERPRINT_PREFIX") to be consistent with *_MIME_TYPE and the other plugins.

@@ +5,3 @@
+	private const string NES_MIME_TYPE = "application/x-nes-rom";
+
+	private const string FINGERPRINT_FDS_PREFIX = "fds";

Ditto.

@@ +26,3 @@
 	}
 
 	private static Game game_for_uri (string uri) throws Error {

Let's rename this function to have consistency within this plugin and to avoid confusion.

@@ +28,2 @@
 	private static Game game_for_uri (string uri) throws Error {
+		var uid = new FingerprintUid (uri, FINGERPRINT_FDS_PREFIX);

Not FDS. :)

@@ +34,3 @@
+			new LocalCover (uri),
+			new GriloCover (media, uid)});
+		var runner = new RetroRunner (uri, uid, { FDS_MIME_TYPE }, MODULE_BASENAME, SUPPORTS_SNAPSHOTTING);

Not FDS either here. :)

@@ +39,3 @@
+	}
+
+	private static Game fds_game_for_uri (string uri) throws Error {

In this function no need to check for the presence of the offset: it's not per se a bad thing to do but it's overengineering as shared-mime-info already checks that for us. We do some useless checks like this check in other plugins, it is also overengineering and should be fixed.

So this function should probably be exactly the same as the NES one except for the MIME type and the prefix.

Created attachment 345178 [details]
The Famicom Disk System was a Japan-exclusive storage device for the Famicom, designed to reduce Nintendo's cost of making copies of games.

Review of attachment 345178 [details]:

The code is good to me but not the commit message, please read the HACKING file to know how to format the commit message.

Created attachment 345221 [details]
nes-plugin.vala: Provide support for fds games

This plugin returns fds(Famicom Disk System) and nes(Nintendo Entertainment System) games.

Review of attachment 345221 [details]:

It's better than before but not yet good enough, but don't worry, it's not easy but it comes with experience. :)

The HACKING file wasn't clear enough on the shortlog's tag and has now been updated, here the correct tag is 'nes'.

FDS is an acronym so --- code aside --- it should be written in upper case.

"This plugin returns fds(Famicom Disk System) and nes(Nintendo Entertainment System) games.": this doesn't state the change made by the commit but the state of the plugin after the commit, the change  here is that the commit makes the plugin list FDS games via the corresponding MIME type.

Review of attachment 345221 [details]:

It's better than before but not yet good enough, but don't worry, it's not easy but it comes with experience. :)

The HACKING file wasn't clear enough on the shortlog's tag and has now been updated, here the correct tag is 'nes'.

FDS is an acronym so --- code aside --- it should be written in upper case.

"This plugin returns fds(Famicom Disk System) and nes(Nintendo Entertainment System) games.": this doesn't state the change made by the commit but the state of the plugin after the commit, the change  here is that the commit makes the plugin list FDS games via the corresponding MIME type.

Created attachment 345401 [details] [review]
nes: Provide support for FDS games

This commit makes the plugin to list FDS games via the corresponding MIME type.

The problem is that the plugin lists only NES games, so we extended the plugin so as to provide support for FDS games by  listing FDS games.

Review of attachment 345401 [details] [review]:

"The problem is that..." the problem is explained in the bug entry, in that case it is obvious enough: detailing would have been useful for a complex solution or to explain what each commit does in a series of commits.

I'll just remove this line and commit the patch, thanks.

Attachment 345401 [details] pushed as 2305175 - nes: Provide support for FDS games