Should we mark some modules as private before v1?

[marcandre]

When reviewing #8274 I realized that it might be a good idea to mark some internal modules and classes as "@api private" before v1. Or is that too late or futile?

[deivid-rodriguez]

I believe it's good idea. If a whole review of all modules and classes is deemed like it would delay the release too much, at least mark those modules where privateness is straightforward. It might flexibilize future refactorings :)

[marcandre]

@bbatsov should I work into this?

[bbatsov]

Yeah, that's a good idea.

[pirj]

Makes total sense: if not done otherwise, we'll have to refrain from refactoring and keep all the internals intact to avoid breakages in custom extensions using this internals directly. At least until we hit 2.0, in ~year from now.

I was thinking of adding a suffix to all methods and running well-known extensions against this. All the breakages will remain in the public API, all the rest can be made private and/or marked as such.

Do you want me to tackle this?

[pirj]

Actually, @jonatas, it seems to be a great way to do with ffast. Unfortunately, I don't have much experience with using it. Would you give it a shot?

[marcandre]

I pushed my branch as #8278, I made some progress but didn't quite finish...

[marcandre]

I was thinking of adding a suffix to all methods

I'm 👎 on that. It makes the codebase ugly. For anyone already using our "private" methods, that's instant breakage when it might never have to break.

[pirj]

It makes the codebase ugly.

Sorry, I didn't express myself clearly. We do this temporarily, to separate those that are never used by well-known extensions.

If we only mark what is an internal implementation detail with @api private and don't make it private, we postpone the problem. When we refactor, it will break extensions somewhere between 1.0 and 2.0.

I would personally prefer it to break earlier to avoid gem 'rubocop', '~> 1.0', '< 1.3' dance. For the end-users it means they won't be able to bump, e.g. rubocop-rails separately from rubocop-minitest, because one depends on < 1.3, and another on >= 1.3.

[marcandre]

We do this temporarily

Ah! Sure, for known "clients" that could be worthwhile to discover issues.

[jonatas]

Ouch! sorry for being late here @pirj, the idea is very simple through the Fastfile.

I didn't test but it would be something like:

Fast.shortcut(:mark_api_private) do
  ruby_files_from(ARGV.last) do |file|
    replace_all('class', file) do |node|
       insert_before(node.loc.expression, "# @api private\n")
    end
  end
end

and then execute fast .mark_api_private path/with/ruby/files

But looking the PR probably it would make some failures as we have several mixes of inner classes with modules.