nenad-vujicic left a comment (openstreetmap/openstreetmap-website#5815)
> Why do we need versions to sort comments? Comments already have IDs that can
> be used for ordering them and deleting some of them doesn't change that - a
> sequence with gaps is just as ordered as one without them...
Yes
tomhughes left a comment (openstreetmap/openstreetmap-website#5815)
Why do we need versions to sort comments? Comments already have IDs that can be
used for ordering them and deleting some of them doesn't change that - a
sequence with gaps is just as ordered as one without them...
--
Reply to
nenad-vujicic left a comment (openstreetmap/openstreetmap-website#5815)
> So what's the `note_comment_id` in the versions table for?
`note_comment_id` is the ID of the `note_comment` from which the `note_version`
is generated. I am using it for ordering note comments (reconstructed from
`note_c
NeatNit left a comment (openstreetmap/openstreetmap-website#5840)
@AntonKhorev Can I ping you to review this? I don't expect to need more changes.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5840#issuecomment-2807334975
You ar
@1ec5 commented on this pull request.
> @@ -10,9 +10,7 @@ atom_feed(:language => I18n.locale, :schema_date => 2009,
feed.logo image_url("mag_map-rss2.0.png")
feed.rights :type => "xhtml" do |xhtml|
-xhtml.a :href => "https://creativecommons.org/licenses/by-sa/2.0/"; do |a|
- a.i
tomhughes left a comment (openstreetmap/openstreetmap-website#5815)
So what's the `note_comment_id` in the versions table for?
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5815#issuecomment-2807249264
You are receiving this bec
nenad-vujicic left a comment (openstreetmap/openstreetmap-website#5815)
> I mean we still haven't finished implementing the previously agreed schema
> changes for notes have we? and now we want to start altering it again..
>
Do you mean removing `event` column?
> I understand the logic of matc
tomhughes left a comment (openstreetmap/openstreetmap-website#5815)
I mean we still haven't finished implementing the previously agreed schema
changes for notes have we? and now we want to start altering it again..
I understand the logic of matching how the geodata is versions, but notes are
no
@tomhughes pushed 2 commits.
8884e56d42c9cfd4ac20e9084597f4375b331d33 Use pre-computed content type in
trace#xml_file
3b792c2892acbff2437f138784bb0eb6d907a59e Workaround bug in file 5.46
--
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5930/files/dd57e8db4d8eb
@tomhughes pushed 1 commit.
13a8386008b8d7cddf3b28a9200d4f677b700599 Add frozen_string_literal comments to
ruby files
--
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5932/files/03a09611f9a81a216d8aff53b048b09717eab3f3..13a8386008b8d7cddf3b28a9200d4f677b700599
tomhughes left a comment (openstreetmap/openstreetmap-website#5923)
So I did manage to reproduce the problem you're referring to and it turns out
we don't actually need that code any more as `Rack::Cors` has supported caching
properly for eight years now, by setting `Vary: Origin` which is bette
@tomhughes pushed 1 commit.
317e4807512470453db6cf66ec148a21544d0224 Drop custom CORS rack module
--
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5923/files/2c76e090e0c6498bee2692d0ea4a2b0cf55bdf30..317e4807512470453db6cf66ec148a21544d0224
You are receiving thi
@tomhughes pushed 5 commits.
5037ae2dde6ab2bcfa20835e9b20d2736a847893 Update to omniauth 2.1, puma 6 and
rack 3
35022c47f97be69fd2dab75f7c3dfe1c8bd8af30 Avoid trying to rewind the request
body
ca3b7fd33270ca0d5fc7999f66db47ff0f85 Pass correct headers to requests
83dae8ca11cdc23c86e9e1a9b1
nenad-vujicic left a comment (openstreetmap/openstreetmap-website#5815)
@gravitystorm @tomhughes @AntonKhorev I would really appreciate if you could
make some comments about this PR since it could greatly help me in wrapping up
my efforts from #5904 (notes versioning, editable note tags, removin
etienneJr created an issue (openstreetmap/openstreetmap-website#5934)
### Problem
Following PR #5468, we still have a user in North of France who posts a huge
amount of anonymous notes. Many of them are cryptic or seem to be copy paste
from gmaps. He does not reply to our messages on these note
@tomhughes approved this pull request.
Thanks for the fix - this looks good now.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5902#pullrequestreview-2769191945
You are receiving this because you are subscribed to this thread.
Merged #5902 into master.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5902#event-17273443856
You are receiving this because you are subscribed to this thread.
Message ID:
___
rails-
Merged #5924 into master.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5924#event-17273443876
You are receiving this because you are subscribed to this thread.
Message ID:
___
rails-
tomhughes left a comment (openstreetmap/openstreetmap-website#5924)
Thanks. This looks good to me now, and nobody's had anything to say about the
colours so I see no reason not to merge this.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-we
Merged #5933 into master.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5933#event-17273307606
You are receiving this because you are subscribed to this thread.
Message ID:
___
rails-
tomhughes left a comment (openstreetmap/openstreetmap-website#5933)
Looks good to me, thanks.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5933#issuecomment-2806860845
You are receiving this because you are subscribed to this t
AntonKhorev left a comment (openstreetmap/openstreetmap-website#5923)
If you run this test
https://github.com/openstreetmap/openstreetmap-website/blob/fd7b428007905463e2359ed0823920954c699375/test/integration/cors_test.rb#L4
do you get to this line
https://github.com/cyu/rack-cors/blob/a3b0a90d7
@rkoeze pushed 1 commit.
006dbecba6b7467e5d76ee3d353f03a84dee7070 Revert change to redirects
--
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5895/files/949f87a806c57f7ab5643f0725ab705d6f9d5279..006dbecba6b7467e5d76ee3d353f03a84dee7070
You are receiving this bec
tomhughes left a comment (openstreetmap/openstreetmap-website#5932)
The problem with that is that it will also affect any gems we use so if any of
them try to modify a constant string (for example
https://github.com/chef/ffi-libarchive/pull/90) then what is currently a
deprecation warning will
See
https://github.com/openstreetmap/openstreetmap-website/pull/4792#issuecomment-2697810051.
LWG says that the correct license is ODbL.
You can view, comment on, or merge this pull request online at:
https://github.com/openstreetmap/openstreetmap-website/pull/5933
-- Commit Summary --
* R
tomhughes left a comment (openstreetmap/openstreetmap-website#5923)
Nothing there specifies a class? The class is what ever rack passes up the
stack, and from my debugging that is `Rack::Headers`.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetm
AntonKhorev left a comment (openstreetmap/openstreetmap-website#5923)
`headers` here
https://github.com/openstreetmap/openstreetmap-website/blob/fd7b428007905463e2359ed0823920954c699375/config/initializers/cors.rb#L9
were `Rack::Utils::HeaderHash` before and became `Hash` after this PR.
--
Repl
tomhughes left a comment (openstreetmap/openstreetmap-website#5923)
Yes it downcases everything now but so long as you're using `Rack::Headers`
instead of a plain hash any lookups are also downcased.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstre
AntonKhorev left a comment (openstreetmap/openstreetmap-website#5923)
Previously I got `Cache-Control: no-cache` in responses to for example
`/api/capabilities`. Now I get `cache-control: max-age=0, private,
must-revalidate`.
--
Reply to this email directly or view it on GitHub:
https://github
@tomhughes pushed 4 commits.
20d1b9fa212ea62a372cf2e7f019dfa38bbda971 Update to omniauth 2.1, puma 6 and
rack 3
85a7d5d880cd8cd3dbfc1b4240bf3e24e81a358a Avoid trying to rewind the request
body
66c9fd1ef6d7f8bd5b7a192e37e0fe797cb31d60 Pass correct headers to POST requests
41513983b97895463c605
@tomhughes commented on this pull request.
> @@ -247,7 +247,7 @@ def respond_to_timeout
# To work round this we call rewind on the body here, which is added
# as a filter, to force it to be fetched from Apache into a file.
def fetch_body
-request.body.rewind
+request.body&.rewin
AntonKhorev left a comment (openstreetmap/openstreetmap-website#5923)
I'm talking about response headers.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5923#issuecomment-2804783217
You are receiving this because you are subscrib
@tomhughes commented on this pull request.
> @@ -247,7 +247,7 @@ def respond_to_timeout
# To work round this we call rewind on the body here, which is added
# as a filter, to force it to be fetched from Apache into a file.
def fetch_body
-request.body.rewind
+request.body&.rewin
tomhughes left a comment (openstreetmap/openstreetmap-website#5923)
I assume you're thinking about
https://github.com/rack/rack/blob/main/UPGRADE-GUIDE.md#response-headers-must-be-lower-case
which is technically about response headers though the same is essentially
true for request headers.
I
tomhughes left a comment (openstreetmap/openstreetmap-website#5923)
Why would it not be if a request arrives with that header? Are you suggesting
that rack 3 has changed how it present request headers or something?
--
Reply to this email directly or view it on GitHub:
https://github.com/openstr
AntonKhorev left a comment (openstreetmap/openstreetmap-website#5923)
Is `if headers["Access-Control-Allow-Origin"]` ever going to be true in
[config/initializers/cors.rb](https://github.com/openstreetmap/openstreetmap-website/blob/fd7b428007905463e2359ed0823920954c699375/config/initializers/cors
tomhughes left a comment (openstreetmap/openstreetmap-website#5932)
To explain what made me suggest this upgrading to Fedora 42 and hence ruby 3.4
led to quite a lot of deprecation warnings almost all of which (apart from the
ffi-libarchive ones) come down to controller tests which make a POST r
tomhughes left a comment (openstreetmap/openstreetmap-website#5923)
> Does this work now, with lowercase header keys?
Am I missing some context here? What "lowercase header keys" are you referring
to?
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/opens
AntonKhorev left a comment (openstreetmap/openstreetmap-website#5923)
Does this work now, with lowercase header keys?
https://github.com/openstreetmap/openstreetmap-website/blob/fd7b428007905463e2359ed0823920954c699375/config/initializers/cors.rb#L10
--
Reply to this email directly or view it on
gravitystorm left a comment (openstreetmap/openstreetmap-website#5932)
>From https://bugs.ruby-lang.org/issues/20205
> And any application for which all dependencies have been made fully frozen
> string literal compatible can set RUBYOPT="--enable=frozen_string_literal"
> and start immediately
kcne left a comment (openstreetmap/openstreetmap-website#5761)
@tomhughes, @AntonKhorev when you find time to look at this again, just let me
know if i need to adjust this further. Thank you.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-w
Ruby 4 intends to make string literals frozen by default and to aid transition
it is possible to opt into that behaviour on a per-file basis by adding a
special comment to the top, as this PR does.
Note that ruby 3.4 already warns if you try and modify a non-frozen string
literal and you have d
github-actions[bot] left a comment (openstreetmap/openstreetmap-website#5932)
2 Warnings
:warning:
Number of updated lines of code is too large to
be in one PR. Perhaps it should be separated into two or more?
:
43 matches
Mail list logo
