Skip to content
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

[#4971] Add Ubuntu 18.04 LTS (Bionic) Support #5232

Merged
merged 1 commit into from
Jun 5, 2019

Conversation

garethrees
Copy link
Member

@garethrees garethrees commented May 31, 2019

Relevant issue(s)

Fixes #4971
Fixes #5025

What does this do?

Add Ubuntu 18.04 LTS (Bionic) Support

Implementation notes

Obvs ignoring the hound comments as its a patch that we can hopefully remove as we upgrade further.

  • Check whether 1f2a600 is actually needed
  • Test a few actions in the app
  • Lots of warnings about constant ::Fixnum is deprecated, but we're seeing those in CI anyway

if postgresql_version >= 100000
minvalue = select_value("SELECT seqmin FROM pg_sequence WHERE seqrelid = #{quote(quoted_sequence)}::regclass")
else
minvalue = select_value("SELECT min_value FROM #{quoted_sequence}")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [83/80]

max_pk = select_value("SELECT MAX(#{quote_column_name pk}) FROM #{quote_table_name(table)}")
if max_pk.nil?
if postgresql_version >= 100000
minvalue = select_value("SELECT seqmin FROM pg_sequence WHERE seqrelid = #{quote(quoted_sequence)}::regclass")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [126/80]


if pk && sequence
quoted_sequence = quote_table_name(sequence)
max_pk = select_value("SELECT MAX(#{quote_column_name pk}) FROM #{quote_table_name(table)}")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [104/80]

end

if @logger && pk && !sequence
@logger.warn "#{table} has primary key #{pk} with no default sequence"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [82/80]

module SchemaStatements
# Resets the sequence of a table's primary key to the maximum value.
def reset_pk_sequence!(table, pk = nil, sequence = nil) #:nodoc:
unless pk and sequence

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use && instead of and.

module ActiveRecord
module ConnectionAdapters
module PostgreSQL
module SchemaStatements

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing top-level module documentation comment.

require 'active_record/connection_adapters/postgresql/schema_statements'

#
# Monkey-patch the refused Rails 4.2 patch at https://github.com/rails/rails/pull /31330

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [91/80]

@lizconlan
Copy link

Lots of warnings about constant ::Fixnum is deprecated

I think this will be resolved in 0.34 (drops ruby 2.0 and allows us to upgrade nokogiri to 1.8.5 which appears to include a fix)

@lizconlan
Copy link

Yep upgrading nokogiri fixes most of the deprecation warnings, but there are still some for BigDecimal.new() vs BigDecimal() which should be a quick fix. I'll make a ticket for that

Screen Shot 2019-06-05 at 09 49 01

Copy link

@lizconlan lizconlan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 yay!

@lizconlan
Copy link

Afterthought - I did not capture the error message but I had to upgrade Vagrant to get this to build (I had an older pre v2 version)

@garethrees
Copy link
Member Author

Afterthought - I did not capture the error message but I had to upgrade Vagrant to get this to build (I had an older pre v2 version)

Pre-v2 is pretty old. Although its not really written down anywhere, I've just considered that vagrant (and virtualbox) should be running on the most recent version unless there's a really good version not to do so.

@lizconlan
Copy link

lizconlan commented Jun 5, 2019

vagrant (and virtualbox) should be running on the most recent version

Yeah, more capturing this in case a reuser hits a similar problem long after I've forgotten about this

@mysociety-pusher mysociety-pusher merged commit 7767d2f into develop Jun 5, 2019
@mysociety-pusher mysociety-pusher deleted the 4971-ubuntu-bionic branch June 5, 2019 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing dependency in Ubuntu 18.04 'pdftk' Add Ubuntu 18.04 LTS (Bionic) support to install scripts
4 participants