New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-surface missing public methods in `Jekyll::Document` #5975

Merged
merged 3 commits into from Mar 31, 2017

Conversation

Projects
None yet
4 participants
@ashmaroli
Member

ashmaroli commented Mar 23, 2017

Numerous public methods for an instance of Jekyll::Document got removed (converted to private methods) in 158e026 which should've been put off till v4.0

This came to my attention via this thread at Jekyll Talk, which reported a broken plugin jekyll-multiple-languages

Now that we have a system to easily release backport-hotfixes, this should probably be released as v3.3.2 (IMO).

--
/cc @ayastreb, @parkr

Show outdated Hide outdated lib/jekyll/document.rb Outdated
Show outdated Hide outdated lib/jekyll/document.rb Outdated
merge_data!({
"tags" => Utils.pluralized_array_from_hash(data, "tag", "tags").flatten,
})
end

This comment has been minimized.

@parkr

parkr Mar 24, 2017

Member

Can these be marked as public without moving them around?

@parkr

parkr Mar 24, 2017

Member

Can these be marked as public without moving them around?

This comment has been minimized.

@ashmaroli

ashmaroli Mar 24, 2017

Member

Since all methods after the first declaration of private are marked as private_methods internally, these methods need to be "moved up" to before the declaration, at least in this case, right after :respond_to_missing?.

@ashmaroli

ashmaroli Mar 24, 2017

Member

Since all methods after the first declaration of private are marked as private_methods internally, these methods need to be "moved up" to before the declaration, at least in this case, right after :respond_to_missing?.

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Mar 25, 2017

Member

Any way we could add a note that these methods should be made private in 4.0?

Member

pathawks commented Mar 25, 2017

Any way we could add a note that these methods should be made private in 4.0?

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Mar 26, 2017

Member

Any way we could add a note that these methods should be made private in 4.0?

@pathawks Think the following will do for now. (Even though it'll be included in the auto-generated documentation):

# DEPRECATION: This method will be made 'private' in the next major release.

Then leave a TODO: comment to our Jekyll 4.0 Wishlist as a reminder for future. As we start developing 4.0, we should have Jekyll::Deprecator warn/inform the user about its removal in the near future.

Member

ashmaroli commented Mar 26, 2017

Any way we could add a note that these methods should be made private in 4.0?

@pathawks Think the following will do for now. (Even though it'll be included in the auto-generated documentation):

# DEPRECATION: This method will be made 'private' in the next major release.

Then leave a TODO: comment to our Jekyll 4.0 Wishlist as a reminder for future. As we start developing 4.0, we should have Jekyll::Deprecator warn/inform the user about its removal in the near future.

@parkr

parkr approved these changes Mar 31, 2017

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Mar 31, 2017

Member

@jekyllbot: merge +bug

Member

parkr commented Mar 31, 2017

@jekyllbot: merge +bug

@jekyllbot jekyllbot merged commit 1b1fe27 into jekyll:master Mar 31, 2017

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jekyllbot jekyllbot added bug fix labels Mar 31, 2017

jekyllbot added a commit that referenced this pull request Mar 31, 2017

@ashmaroli ashmaroli deleted the ashmaroli:resurface-methods branch Mar 31, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment