Skip to content

Conversation

@thisisnic
Copy link
Member

No description provided.

@github-actions
Copy link

@github-actions
Copy link

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@thisisnic thisisnic marked this pull request as ready for review January 21, 2022 10:21
@jorisvandenbossche
Copy link
Member

Can you print an example of what json file the scripts outputs eg for the 7.0 release?

@thisisnic
Copy link
Member Author

Can you print an example of what json file the scripts outputs eg for the 7.0 release?

Certainly can. For the main docs, you manually add the dev version in the JS, right?

[
    {
        "name": "7.0 (stable)",
        "version": ""
    },
    {
        "name": "6.0",
        "version": "6.0/"
    },
    {
        "name": "5.0",
        "version": "5.0/"
    },
    {
        "name": "4.0",
        "version": "4.0/"
    },
    {
        "name": "3.0",
        "version": "3.0/"
    },
    {
        "name": "2.0",
        "version": "2.0/"
    },
    {
        "name": "1.0",
        "version": "1.0/"
    }
]

@jorisvandenbossche
Copy link
Member

For the main docs, you manually add the dev version in the JS, right?

Ah, no, it would actually be good this is handled by the script as well. I added it manually last week directly in the hosted sources (apache/arrow-site#177) to get the dropdown working for the new nightly docs that were added (originally when adding the dropdown, we didn't yet have dev docs). But I should actually have ported that back to the main version of the json file in the apache/arrow repo)

@thisisnic
Copy link
Member Author

thisisnic commented Jan 21, 2022

For the main docs, you manually add the dev version in the JS, right?

Ah, no, it would actually be good this is handled by the script as well. I added it manually last week directly in the hosted sources (apache/arrow-site#177) to get the dropdown working for the new nightly docs that were added (originally when adding the dropdown, we didn't yet have dev docs). But I should actually have ported that back to the main version of the json file in the apache/arrow repo)

Just pushed a commit which now results in this; is this what you'd expect? I also noticed that dev has no / at the end - does it matter? I construct URLs a bit differently in the R version.

[
    {
        "name": "8.0 (dev)",
        "version": "dev"
    },
    {
        "name": "7.0 (stable)",
        "version": ""
    },
    {
        "name": "6.0",
        "version": "6.0/"
    },
    {
        "name": "5.0",
        "version": "5.0/"
    },
    {
        "name": "4.0",
        "version": "4.0/"
    },
    {
        "name": "3.0",
        "version": "3.0/"
    },
    {
        "name": "2.0",
        "version": "2.0/"
    },
    {
        "name": "1.0",
        "version": "1.0/"
    }
]

@jorisvandenbossche
Copy link
Member

Yes, that looks good!

@thisisnic thisisnic force-pushed the ARROW-15366_increment_versions branch from 1914e9e to 5ef27e9 Compare February 2, 2022 15:37
@thisisnic
Copy link
Member Author

Thanks for the feedback and suggestions here @kou !

Now entirely stuck again on test failures - the CI is showing differences between the expected line in one of the json files and the line that is there.

I've tried removing it entirely in 1 PR (see CI failure here: https://github.com/apache/arrow/runs/5040927092?check_suite_focus=true#step:6:403) and then copying and pasting the annotated "missing" line into the tests in a subsequent PR (see CI failure here: https://github.com/apache/arrow/runs/5040977997?check_suite_focus=true#step:6:405) but it's still showing up as not matching.

I've copied and pasted the two lines and done a diff on them and there's no difference, so I don't know what a sensible next step is here.

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

I've fixed the test failures. I've also fixed versions.json generation when "X.Y (dev)" exists.

@kou
Copy link
Member

kou commented Feb 3, 2022

I've also add missing trailing "/" to dev version.

@kou kou closed this in 360252b Feb 3, 2022
@ursabot
Copy link

ursabot commented Feb 3, 2022

Benchmark runs are scheduled for baseline = c89e67d and contender = 360252b. 360252b is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.26% ⬆️0.09%] ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@thisisnic
Copy link
Member Author

Thanks for finishing off this PR @kou, I was getting pretty frustrated with it, so your help here is much appreciated! :D

kszucs pushed a commit to kszucs/arrow that referenced this pull request Mar 3, 2022
…d non-R version switchers

Closes apache#12212 from thisisnic/ARROW-15366_increment_versions

Lead-authored-by: Nic Crane <thisisnic@gmail.com>
Co-authored-by: Sutou Kouhei <kou@clear-code.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants