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

Refactor code #136

Merged
merged 17 commits into from
Jun 13, 2023
Merged

Refactor code #136

merged 17 commits into from
Jun 13, 2023

Conversation

nobkd
Copy link
Contributor

@nobkd nobkd commented Jun 7, 2023

It's probably the easiest to read the diff of the individual commits instead of comparing everything at once.
The commit names should be reasonably accurate in describing what was changed.

Here is a rough summary of the changes:

  • removing unused imports, ordering, reusing code pieces, formatting md
  • cleaning up spaces, use ' as main quotation mark
  • use format strings instead of old %-formatting
  • storing os-release file data in global variable instead of re-reading the file
  • add action option with default on 'in' to install_packages for code reusing (maybe change function name?)

@asdil12
Copy link
Member

asdil12 commented Jun 11, 2023

There is a merge commit in the list of commits - maybe you can get rid if that one.

@nobkd
Copy link
Contributor Author

nobkd commented Jun 11, 2023

Okay. Then I'll undo the last 3 commits and resolve your comments (in a few hours).
After merging I'll do another pr to change some code pieces you've added previously

opi/__init__.py Outdated Show resolved Hide resolved
@nobkd
Copy link
Contributor Author

nobkd commented Jun 11, 2023

This should resolve the merge conflicts. I've done it this way, because you didn't want to have a Merge commit.

opi/__init__.py
1.
				'alias': mainsec,
				'filename': re.sub(r'\.repo$', '', repo_file),
				'name': cp[mainsec].get('name', mainsec),
				'url': cp.get(mainsec, 'baseurl'),
			}
			if cp.has_option(mainsec, 'gpgkey'):
				repo['gpgkey'] = cp.get(mainsec, 'gpgkey')
2.
		if binary['obs_instance'] == 'LOCAL_REPO':
			existing_repo = repository
		else:
			repo_alias = project.replace(':', '_')
			project_path = project.replace(':', ':/')
			if config.get_key_from_config('use_releasever_var'):
				version = get_version()
				if version:
					# version is None on tw
					repository = repository.replace(version, '$releasever')
			url = f'https://download.opensuse.org/repositories/{project_path}/{repository}/'
			gpgkey = url + 'repodata/repomd.xml.key'
			existing_repo = get_enabled_repo_by_url(url)
3.
			print('This key is still in use by the following remaining repos - removal is NOT recommended:')
			print(' - ' + '\n - '.join([repo['filename'] for repo in repos_using_this_key]))
4.
	if not ask_yes_or_no(f"Do you want to keep the repo '{repo}'?"):
		repo_info = next((r for r in get_repos() if r['filename'] == repo))
5.
	if binary['obs_instance'] not in ('openSUSE', 'LOCAL_REPO'):
		project = f"{binary['obs_instance']} {project}"

opi/plugins/zoom.py
1.
		key_url = 'https://zoom.us/linux/download/pubkey?version=5-12-6'

opi/__init__.py Show resolved Hide resolved
opi/pager.py Outdated Show resolved Hide resolved
@asdil12
Copy link
Member

asdil12 commented Jun 12, 2023

Could you resolve that merge conflicts by running git pull --rebase origin master to rebase onto origin/master?
That way you don't need to add a merge commit.

@nobkd nobkd requested a review from asdil12 June 12, 2023 12:54
opi/__init__.py Outdated Show resolved Hide resolved
test/05_install_from_local_repo.py Outdated Show resolved Hide resolved
nobkd and others added 2 commits June 13, 2023 12:33
Co-authored-by: Dominik Heidler <dominik@heidler.eu>
Co-authored-by: Dominik Heidler <dominik@heidler.eu>
@asdil12 asdil12 merged commit a6f5a66 into openSUSE:master Jun 13, 2023
2 checks passed
@nobkd nobkd deleted the refactor/code branch June 13, 2023 22:35
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.

None yet

2 participants