Opened 20 months ago

Closed 20 months ago

Last modified 18 months ago

#6893 closed change (fixed)

Split out the INIParser class into its own file — at Version 4

Reported by: mjethani Assignee: mjethani
Priority: P3 Milestone:
Module: Core Keywords:
Cc: Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

Description (last modified by mjethani)

Background

The INIParser class is currently internal to lib/filterStorage.js. It is not well documented. It is tested only as part of lib/filterStorage.js.

This is an important class for multiple reasons, most importantly performance. If we are to make changes to the INI file format (see discussion in #6861), this class should firstly be well written and well documented. We are also migrating to ES2015 classes on a file-by-file basis (see #6741), and it would be easier to manage the changes if large files like lib/filterStorage.js are gradually split up into smaller files.

What to change

  1. Move the definition of INIParser into lib/iniParser.js
  2. Convert to ES2015 class
  3. Make properties not needed by other modules "private" (use an underscore prefix)
  4. Add JSDoc comments

Additional notes

The unit testing for this class is currently covered by test/filterStorage_readwrite.js. This is OK, there's no need to write separate tests, but it may make sense to do so in the future, especially if there are changes to the file format.

Hints for testers

This is a development/refactoring task and there are no specific things to test, as there is no expected change in behavior. Just make sure that there are no regressions, i.e. filters are saved and loaded correctly to and from the patterns.ini file.

Change History (4)

comment:1 Changed 20 months ago by mjethani

  • Description modified (diff)

comment:2 Changed 20 months ago by abpbot

A commit referencing this issue has landed:
Issue 6893, 6741 - Split out INIParser into its own file

comment:3 Changed 20 months ago by mjethani

  • Description modified (diff)
  • Resolution set to fixed
  • Status changed from new to closed

comment:4 Changed 19 months ago by mjethani

  • Description modified (diff)
Note: See TracTickets for help on using tickets.