GitHub VCS Bugfixes (#21)

* Bugfixes after initial functional usage

* Fixing pyproject python troves

* Updating readme template list

docs/source/usage.md

@@ -442,7 +442,7 @@ This could be implemented as a staticmethod on the recipe object or similar, but
 
 You can also customise the environment initialisation if you need to override specifics of the configuration however this is not recommended as it can cause complex issues with the templates/handling.
 
-The default implementation defines the loader to use ``_PkgResourcesTemplateLoader`` to allow for package resources type loading on the environment (see examples above), but other parameters could be configured/changed
+The default implementation defines the loader to use ``_PkgResourcesTemplateLoader`` to allow for package resources type loading on the environment (see examples above), but other parameters could be configured/changed by setting the ``NSKIT_MIXER_JINJA_ENVIRONMENT_FACTORY`` environment variable to the name of the entrypoint.
 
 !!! warning
 

pyproject.toml

@@ -15,11 +15,11 @@ classifiers = [
         "Environment :: Console",
         "Topic :: Software Development :: Libraries :: Python Modules",
         "Programming Language :: Python :: 3",
-        "Programming Language :: Python :: 3.7",
         "Programming Language :: Python :: 3.8",
         "Programming Language :: Python :: 3.9",
         "Programming Language :: Python :: 3.10",
         "Programming Language :: Python :: 3.11",
+        "Programming Language :: Python :: 3.12",
         "Intended Audience :: Developers",
     	"License :: OSI Approved :: MIT License",
         "Operating System :: Microsoft :: Windows",

src/nskit/mixer/components/license_file.py

@@ -111,7 +111,7 @@ def get_license_content(context: Dict[str, Any]):
             LicenseOptionsEnum.BSD_3_Clause,
             LicenseOptionsEnum.MIT,
         ]:
-            content = content.replace('[year]', '{{license_year}}').replace('[fullname]', '{{name}} Developers')
+            content = content.replace('[year]', '{{license_year}}').replace('[fullname]', '{{repo.name}} Developers')
         context['license_year'] = context.get('license_year', date.today().year)
         return content
 

src/nskit/recipes/python/ingredients/docs/license.md.template

@@ -1,7 +1,7 @@
 {% if license %}
 # License
 
-``{{repo.name}}`` is licensed under the {{license | upper }} License
+``{{repo.name}}`` is licensed under the {{license.value | upper }} License
 {% raw %}
 ```
 {%

src/nskit/recipes/python/ingredients/noxfile.py.template

@@ -1,6 +1,7 @@
 import os
 from pathlib import Path
 from platform import platform, python_version
+import sys
 
 if sys.version_info.major <=3 and sys.version_info.minor < 11:
     import tomli as tomllib  # pyright: ignore [reportMissingImports]

src/nskit/recipes/python/ingredients/pyproject.toml.template

@@ -15,11 +15,11 @@ classifiers = [
         "Environment :: Console",
         "Topic :: Software Development :: Libraries :: Python Modules",
         "Programming Language :: Python :: 3",
-        "Programming Language :: Python :: 3.7",
         "Programming Language :: Python :: 3.8",
         "Programming Language :: Python :: 3.9",
         "Programming Language :: Python :: 3.10",
         "Programming Language :: Python :: 3.11",
+        "Programming Language :: Python :: 3.12",
         "Intended Audience :: Developers",
     	"License :: OSI Approved :: MIT License",
         "Operating System :: Microsoft :: Windows",

src/nskit/recipes/python/ingredients/readme.md.template

@@ -25,9 +25,10 @@ Tests can be added to the tests folder
 ### CI
 {% block CI %}
 There are a set of CI checks:
-    * lint: ``nox -t lint``
-    * test: ``nox -t test``
-    * build: ``nox -t build``
+
+* lint: ``nox -t lint``
+* test: ``nox -t test``
+* build: ``nox -t build``
 
 specific subfolders can be passed to the test tag in nox: ``nox -t test -- <subfolder1> <subfolder2>``
 

src/nskit/vcs/providers/abstract.py

@@ -20,6 +20,13 @@ def get_remote_url(self, repo_name: str) -> HttpUrl:
         """Get the remote url for a repo."""
         raise NotImplementedError()
 
+    def get_clone_url(self, repo_name: str) -> HttpUrl:
+        """Get the clone URL.
+
+        This defaults to the remote url unless specifically implemented.
+        """
+        return self.get_remote_url(repo_name)
+
     @abstractmethod
     def delete(self, repo_name: str):
         """Delete a repo."""

src/nskit/vcs/providers/github.py

@@ -18,7 +18,7 @@
 class GithubRepoSettings(BaseConfiguration):
     """Github Repo settings."""
     private: bool = True
-    has_issue: Optional[bool] = None
+    has_issues: Optional[bool] = None
     has_wiki: Optional[bool] = None
     has_downloads: Optional[bool] = None
     has_projects: Optional[bool] = None
@@ -36,7 +36,7 @@ class GithubSettings(VCSProviderSettings):
     """
     model_config = SettingsConfigDict(env_prefix='GITHUB_', env_file='.env')
     interactive: bool = Field(False, description='Use Interactive Validation for token')
-    url: HttpUrl = "https://github.com"
+    url: HttpUrl = "https://api.github.com"
     organisation: Optional[str] = Field(None, description='Organisation to work in, otherwise uses the user for the token')
     token: SecretStr = Field(None, validate_default=True, description='Token to use for authentication, falls back to interactive device authentication if not provided')
     repo: GithubRepoSettings = GithubRepoSettings()
@@ -69,39 +69,47 @@ class GithubRepoClient(RepoClient):
     def __init__(self, config: GithubSettings):
         """Initialise the client."""
         self._config = config
-        self._github = GhApi(token=self._config.token.get_secret_value(), gh_host=self._config.url)
+        self._github = GhApi(token=self._config.token.get_secret_value(), gh_host=str(self._config.url).rstrip('/'))
         # If the organisation is set, we get it, and assume that the token is valid
         # Otherwise default to the user
         if self._config.organisation:
             try:
                 self._github.orgs.get(self._config.organisation)
                 self._org_type = GithubOrgType.org
             except HTTP404NotFoundError:
-                self._github.user.get_by_username(self._config.organisation)
+                self._github.users.get_by_username(self._config.organisation)
                 self._org_type = GithubOrgType.user
         else:
-            self._config.organisation = self._github.get_authenticated()['login']
+            self._config.organisation = self._github.users.get_authenticated()['login']
             self._org_type = GithubOrgType.user
 
     def create(self, repo_name: str):
         """Create the repo in the user/organisation."""
-        return self._github.repos.create_in_org(
-            self._config.organisation,
-            name=repo_name,
-            private=self._config.repo.private,
-            has_issues=self._config.repo.has_issues,
-            has_wiki=self._config.repo.has_wiki,
-            has_downloads=self._config.repo.has_downloads,
-            has_projects=self._config.repo.has_projects,
-            allow_squash_merge=self._config.repo.allow_squash_merge,
-            allow_merge_commit=self._config.repo.allow_merge_commit,
-            allow_rebase_merge=self._config.repo.allow_rebase_merge,
-            auto_init=self._config.repo.auto_init,
-            delete_branch_on_merge=self._config.repo.delete_branch_on_merge
-        )
+        kwargs = {
+            'name': repo_name,
+            'private': self._config.repo.private,
+            'has_issues': self._config.repo.has_issues,
+            'has_wiki': self._config.repo.has_wiki,
+            'has_downloads': self._config.repo.has_downloads,
+            'has_projects': self._config.repo.has_projects,
+            'allow_squash_merge': self._config.repo.allow_squash_merge,
+            'allow_merge_commit': self._config.repo.allow_merge_commit,
+            'allow_rebase_merge': self._config.repo.allow_rebase_merge,
+            'auto_init': self._config.repo.auto_init,
+            'delete_branch_on_merge': self._config.repo.delete_branch_on_merge
+        }
+        if self._org_type == GithubOrgType.org:
+            self._github.repos.create_in_org(self._config.organisation, **kwargs)
+        else:
+            self._github.repos.create_for_authenticated_user(**kwargs)
 
     def get_remote_url(self, repo_name: str) -> HttpUrl:
         """Get the remote url for the repo."""
+        if self.check_exists(repo_name):
+            return self._github.repos.get(self._config.organisation, repo_name)['html_url']
+
+    def get_clone_url(self, repo_name: str) -> HttpUrl:
+        """Get the clone url for the repo."""
         if self.check_exists(repo_name):
             return self._github.repos.get(self._config.organisation, repo_name)['clone_url']
 

src/nskit/vcs/repo.py

@@ -99,6 +99,11 @@ def url(self):
         """Get the remote url."""
         return self.provider_client.get_remote_url(self.name)
 
+    @property
+    def clone_url(self):
+        """Get the remote url."""
+        return self.provider_client.get_clone_url(self.name)
+
     def create(self):
         """Create the repo."""
         if not self.exists:
@@ -135,8 +140,8 @@ def clone(self):
         if not self.exists_locally:
             if not self.local_dir.parent.exists():
                 self.local_dir.parent.mkdir(exist_ok=True, parents=True)
-            if self.url:
-                self._git_repo_cls.clone_from(url=self.url, to_path=str(self.local_dir))
+            if self.clone_url:
+                self._git_repo_cls.clone_from(url=self.clone_url, to_path=str(self.local_dir))
 
     def pull(self, remote=DEFAULT_REMOTE):
         """Pull the repo from the remote (defaults to origin)."""

tests/functional/test_license_file.py

@@ -24,7 +24,7 @@ def test_write_license(self):
             pre = [u for u in Path.cwd().glob('*')]
             license_file = LicenseFile()
             try:
-                resp = license_file.write(Path('.'), {'license':'mit', 'name': 'test_repo2'})
+                resp = license_file.write(Path('.'), {'license':'mit', 'repo': {'name': 'test_repo2'}})
             except HTTP403ForbiddenError:
                 self.skip_if_rate_limited()
             post = [u for u in Path.cwd().glob('*')]
@@ -40,7 +40,7 @@ def test_write_license(self):
     def test_dry_run_license(self):
         license_file = LicenseFile()
         try:
-            resp = license_file.dryrun(Path('.'), {'license':'mit', 'name': 'test_repo2'})
+            resp = license_file.dryrun(Path('.'), {'license':'mit', 'repo': {'name': 'test_repo2'}})
         except HTTP403ForbiddenError:
             self.skip_if_rate_limited()
 
@@ -52,8 +52,8 @@ def test_validate_license_ok(self):
         with ChDir():
             license_file = LicenseFile()
             try:
-                license_file.write(Path('.'), {'license':'mit', 'name': 'test_repo2'})
-                missing, errors, ok = license_file.validate(Path('.'), {'license':'mit', 'name': 'test_repo2'})
+                license_file.write(Path('.'), {'license':'mit', 'repo': {'name': 'test_repo2'}})
+                missing, errors, ok = license_file.validate(Path('.'), {'license':'mit', 'repo': {'name': 'test_repo2'}})
             except HTTP403ForbiddenError:
                 self.skip_if_rate_limited()
             self.assertEqual(missing, [])
@@ -64,7 +64,7 @@ def test_validate_license_missing(self):
         with ChDir():
             license_file = LicenseFile()
             try:
-               missing, errors, ok = license_file.validate(Path('.'), {'license':'mit', 'name': 'test_repo2'})
+               missing, errors, ok = license_file.validate(Path('.'), {'license':'mit', 'repo': {'name': 'test_repo2'}})
             except HTTP403ForbiddenError:
                 self.skip_if_rate_limited()
             self.assertEqual(missing, [Path('LICENSE')])
@@ -77,7 +77,7 @@ def test_validate_license_error(self):
             # License doesn't have the year fullname replacement
             try:
                 license_file.write(Path('.'), {'license': 'mpl-2.0'})
-                missing, errors, ok = license_file.validate(Path('.'), {'license':'mit', 'name': 'test_repo2'})
+                missing, errors, ok = license_file.validate(Path('.'), {'license':'mit', 'repo': {'name': 'test_repo2'}})
             except HTTP403ForbiddenError:
                 self.skip_if_rate_limited()
             self.assertEqual(missing, [])
@@ -87,7 +87,7 @@ def test_validate_license_error(self):
     def test_override_year(self):
         license_file = LicenseFile()
         try:
-            resp = license_file.dryrun(Path('.'), {'license':'mit', 'name': 'test_repo2', 'license_year': 1880})
+            resp = license_file.dryrun(Path('.'), {'license':'mit', 'repo': {'name': 'test_repo2'}, 'license_year': 1880})
         except HTTP403ForbiddenError:
             self.skip_if_rate_limited()
 
@@ -100,7 +100,7 @@ def test_each_license_render_content(self):
             with self.subTest(license=license_name):
                 # Test that it renders content
                 try:
-                    license_content = LicenseFile().render_content(context={'license': license_name, 'name': 'test_repo_name'})
+                    license_content = LicenseFile().render_content(context={'license': license_name, 'repo': {'name': 'test_repo_name'}})
                 except HTTP403ForbiddenError:
                     self.skip_if_rate_limited()
                 self.assertIsNotNone(license_content)

tests/unit/test_mixer/test_components/test_license_file.py

@@ -50,12 +50,12 @@ class LicenseFileTestCase(unittest.TestCase):
     def test_get_license_content_cache(self, GhApi):
         # We need to clear the cache here on the test
         _get_license_content.cache_clear()
-        self.assertEqual(f'{date.today().year} test_repo Developers abc', LicenseFile().render_content({'license': 'mit', 'name': 'test_repo'}))
+        self.assertEqual(f'{date.today().year} test_repo Developers abc', LicenseFile().render_content({'license': 'mit', 'repo':{'name': 'test_repo'}}))
         GhApi.assert_called_once_with()
         GhApi().licenses.get.assert_called_once_with('mit')
         GhApi().licenses.get.reset_mock()
         GhApi.reset_mock()
-        self.assertEqual(f'{date.today().year} test_repo2 Developers abc', LicenseFile().render_content({'license': 'mit', 'name': 'test_repo2'}))
+        self.assertEqual(f'{date.today().year} test_repo2 Developers abc', LicenseFile().render_content({'license': 'mit', 'repo':{'name': 'test_repo2'}}))
         GhApi.assert_not_called()
         GhApi().licenses.get.assert_not_called()
 
@@ -105,7 +105,7 @@ def test_write_license(self, GhApi):
             self.assertFalse(Path('UNLICENSE').exists())
             pre = [u for u in Path.cwd().glob('*')]
             license_file = LicenseFile()
-            resp = license_file.write(Path('.'), {'license':'mit', 'name': 'test_repo2'})
+            resp = license_file.write(Path('.'), {'license':'mit', 'repo':{'name': 'test_repo2'}})
             post = [u for u in Path.cwd().glob('*')]
             self.assertTrue(Path('LICENSE').exists())
             self.assertFalse(Path('COPYING').exists())
@@ -117,15 +117,15 @@ def test_write_license(self, GhApi):
     @mock_gh_api
     def test_dry_run_license(self, GhApi):
         license_file = LicenseFile()
-        resp = license_file.dryrun(Path('.'), {'license':'mit', 'name': 'test_repo2'})
+        resp = license_file.dryrun(Path('.'), {'license':'mit', 'repo':{'name': 'test_repo2'}})
         self.assertEqual(resp, {Path('LICENSE'): f'{date.today().year} test_repo2 Developers abc'})
 
     @mock_gh_api
     def test_validate_license_ok(self, GhApi):
         with ChDir():
             license_file = LicenseFile()
-            license_file.write(Path('.'), {'license':'mit', 'name': 'test_repo2'})
-            missing, errors, ok = license_file.validate(Path('.'), {'license':'mit', 'name': 'test_repo2'})
+            license_file.write(Path('.'), {'license':'mit', 'repo':{'name': 'test_repo2'}})
+            missing, errors, ok = license_file.validate(Path('.'), {'license':'mit', 'repo':{'name': 'test_repo2'}})
             self.assertEqual(missing, [])
             self.assertEqual(errors, [])
             self.assertEqual(ok, [Path('LICENSE')])
@@ -134,7 +134,7 @@ def test_validate_license_ok(self, GhApi):
     def test_validate_license_missing(self, GhApi):
         with ChDir():
             license_file = LicenseFile()
-            missing, errors, ok = license_file.validate(Path('.'), {'license':'mit', 'name': 'test_repo2'})
+            missing, errors, ok = license_file.validate(Path('.'), {'license':'mit', 'repo':{'name': 'test_repo2'}})
             self.assertEqual(missing, [Path('LICENSE')])
             self.assertEqual(errors, [])
             self.assertEqual(ok, [])
@@ -145,15 +145,15 @@ def test_validate_license_error(self, GhApi):
             license_file = LicenseFile()
             # License doesn't have the year fullname replacement
             license_file.write(Path('.'), {'license': 'mpl-2.0'})
-            missing, errors, ok = license_file.validate(Path('.'), {'license':'mit', 'name': 'test_repo2'})
+            missing, errors, ok = license_file.validate(Path('.'), {'license':'mit', 'repo':{'name': 'test_repo2'}})
             self.assertEqual(missing, [])
             self.assertEqual(errors, [Path('LICENSE')])
             self.assertEqual(ok, [])
 
     @mock_gh_api
     def test_override_year(self, GhApi):
         license_file = LicenseFile()
-        resp = license_file.dryrun(Path('.'), {'license':'mit', 'name': 'test_repo2', 'license_year': 2022})
+        resp = license_file.dryrun(Path('.'), {'license':'mit', 'repo':{'name': 'test_repo2'}, 'license_year': 2022})
         self.assertEqual(resp, {Path('LICENSE'): '2022 test_repo2 Developers abc'})
 
     def test_license_filename(self):
@@ -217,7 +217,7 @@ def test_each_license_render_content(self, GhApi):
         for license_name in LicenseOptionsEnum:
             with self.subTest(license=license_name):
                 # Test that it renders content
-                license_content = LicenseFile().render_content(context={'license': license_name, 'name': 'test_repo_name'})
+                license_content = LicenseFile().render_content(context={'license': license_name, 'repo':{'name': 'test_repo_name'}})
                 self.assertIsNotNone(license_content)
                 self.assertNotIn('[year]', license_content)
                 self.assertNotIn('[fullname]', license_content)