-
Notifications
You must be signed in to change notification settings - Fork 69
User Repositories: Adapt autoyast conversion #2458
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
Conversation
dgdavid
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementation-wise, I don't have strong opinions. The code looks solid and appears thoroughly tested. Approving it.
| software["packages"] = profile["software"].fetch_as_array("packages") | ||
| end | ||
| if profile["add-on"] | ||
| repos = process_repos |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NP: I do not know if that logic should live here. A bit disconnected of that part currently. But I'd at least rename it to fecth_addon_repos to make it more readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, idea of method is to translate autoyast entries into agama profile ones...not sure if fetch is the right verb for it
| res = {} | ||
| res["url"] = repo["media_url"] | ||
| # alias is mandatory to craft one if needed | ||
| res["alias"] = repo["alias"] || "autoyast_#{index}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: we will sill using "autoyast" naming?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, it is conversion from autoyast profile. But we can name it whatever we want...I am just not sure about better name
joseivanlopez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I miss a changelog entry.
| end | ||
|
|
||
| context "when a list of add-ons are provided" do | ||
| it "includes the list of repositoriess under 'software.extraRepositories'" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: repositoriess
joseivanlopez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Prepare to release Agama 16: * #1868 * #2347 * #2356 * #2373 * #2393 * #2402 * #2404 * #2406 * #2408 * #2409 * #2410 * #2411 * #2412 * #2413 * #2414 * #2415 * #2416 * #2417 * #2418 * #2419 * #2420 * #2421 * #2422 * #2423 * #2424 * #2425 * #2426 * #2427 * #2428 * #2431 * #2433 * #2434 * #2436 * #2437 * #2438 * #2439 * #2440 * #2441 * #2442 * #2443 * #2445 * #2446 * #2450 * #2451 * #2452 * #2453 * #2454 * #2455 * #2456 * #2457 * #2458 * #2460 * #2461 * #2462 * #2463 * #2464 * #2465 * #2466 * #2467 * #2468 * #2469 * #2470 * #2471 * #2472 * #2473 * #2474 * #2475 * #2476 * #2478 * #2479 * #2480 * #2482 * #2483 * #2484 * #2485 * #2487 * #2488 * #2489 * #2490 * #2491 * #2493 * #2494 * #2495 * #2496 * #2497 * #2498 * #2499 * #2502 * #2505 * #2507 * #2509 * #2511 * #2512 * #2513 * #2515 * #2516 * #2517 * #2518 * #2520 * #2523 * #2524 * #2525
Adapt autoyast conversion to new extraRepositories implemented at #2424