Repository navigation
refactor(api)!: one name per meaning across the public API - #59
Merged
Merged
Conversation
The public names that said something other than what the member does, or that used one word for two things: - Scan<TRecord>.AggAsync(a => ...) and AggAsync<TResult>(a => (...)) are AggregateAsync. The name joins the terminal answers (CountAsync, SumAsync) without an abbreviation, and AggregateAsync<T, TAggregator, TState>, an aggregator of one's own, stays an overload of the same name. VX1010 and VX1011 look for the new name and still leave that overload alone: VX1010 wants one type argument and a lambda that returns a tuple, VX1011 a name without type arguments. - Statistics on a scan, a projection and an aggregation is Metrics, and ScanStatistics and GroupStatistics are ScanMetrics and GroupMetrics. Statistics now only names what describes the data: a file's statistics, zone maps, VerifyStatistics. The internal counters behind the metrics, ScanMetrics and DatasetScanMetrics, become ScanCounters and DatasetScanCounters, and the members that hold them Counters. - VortexSession.AppendAsync(path), which returns a writer, is OpenWriterAsync, the pair of CreateWriter. Append now always writes rows, as it does on a dataset. - VortexDataset.ObjectsAsync, RemoveAsync and ReplaceAsync are ListObjectsAsync, RemoveObjectsAsync and ReplaceObjectsAsync, set apart from DeleteAsync and UpdateAsync, which change rows. - KeyCursor<TKey>.KeyCountAsync counted the entries of the current key, not the keys. It is CountAtKeyAsync. - VortexSchema.TryGetField(utf8, out int index) returned an index. It is IndexOf(ReadOnlySpan<byte>), -1 when no field matches, like IndexOf(string). The guide, the design documents, the samples and the tests follow, and the public surface record is replaced.
… and switches The members that broke a rule the rest of the API keeps: - As is the same data in another form, still borrowed from its batch. Column<T>.Canonical() is AsCanonical(), so the four forms of ColumnEncoding read alike (AsCanonical, AsDictionary, AsRunEnd, AsConstant), and Storage() and Storage<TStorage>(), a view of the stored values, are AsStorage(). To stays an owned copy, and Get a value computed or read: VortexFileRepair.ValidLengthAsync is GetValidLengthAsync, like GetLayoutAsync. - No abbreviation: KeyCursor<TKey>.PrevAsync and PrevKeyAsync are PreviousAsync and PreviousKeyAsync, and SeekOp is SeekMode, the parameter of SeekAsync with it. - One word, one meaning: CompactionJob.Objects, a list of keys where Objects is a count everywhere else, is ObjectKeys. GroupKeyPlan.Values, a count where Values is the data everywhere else, is ValueRange. VortexDataset.ClusteringKeyPaths is ClusteringKey, as in DatasetOptions. - A quantity of bytes ends in Bytes: VortexOpenOptions.MaxDecompressedSize, MaxBatchDecompressedSize and InitialReadSize, and VortexLimits.DefaultMaxDecompressedSize, end in Bytes, and SegmentCache.Capacity and Size are CapacityBytes and HeldBytes. The internal read options and the messages that name them follow. - A switch is a verb: ScanOptions.Pruning is UseStatistics, the pair of UseIndexes, VortexWriteOptions.Statistics is WriteStatistics, and CompactionSchedule.Leases is UseLeases. Counts keep their names (RowCount, ZoneCount, EntryCount next to Rows and Entries), and TimeUnit its plurals. The guide, the design documents, the samples, the bench and the tests follow, and the public surface record is replaced.
The public API design gains a section on names, next to rule 6, which decides the Async suffix: what As, To and Get return and what a method without a prefix returns, a verb for what runs, one meaning per word (statistics describe the data, metrics count what a query did, Create, Open and Append, rows against objects on a dataset), names written out but for Sym, Expr and DType, Bytes for a quantity of bytes, a verb for a switch, and a plural for a unit that counts a quantity. A table lists every member the two renaming commits changed, before and now, and the query design's migration table and the guide's point to it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Give the public API one vocabulary. A review of every public member found names that said something other than what the member does, words used for two things (
Statisticsfor the data and for what a query did,Appendfor writing rows and for opening a writer), abbreviations, and a few members that broke rules the rest of the API keeps. The branch renames those members, writes the rules down in the public API design, and lists every rename for callers.What it brings
AggregateAsync, next toCountAsyncandSumAsync, and an aggregator of one's own stays an overload of the same name. What a query did isMetrics(ScanMetrics,GroupMetrics), and statistics only describe the data. A writer over an existing file comes fromOpenWriterAsync, the pair ofCreateWriter. A dataset lists, removes and replaces objects (ListObjectsAsync,RemoveObjectsAsync,ReplaceObjectsAsync), and deletes and updates rows.CountAtKeyAsynccounts the entries of the current key, andIndexOf(utf8)returns an index, -1 when no field matches.Asis the same data in another form, still borrowed, so the four forms of a column read alike (AsCanonical,AsDictionary,AsRunEnd,AsConstant) andAsStoragejoins them.Tois an owned copy, andGeta value computed or read (GetValidLengthAsync).PreviousAsync,PreviousKeyAsync,SeekMode), quantities of bytes that say so (MaxDecompressedBytes,InitialReadBytes,CapacityBytes,HeldBytes), and switches that are verbs (UseStatistics,WriteStatistics,UseLeases).Risks and tests
dotnet build Vorticity.slnx -c Releasepasses with no warning.Vorticity.Testspasses in full, 7 311 tests, with 4 skipped by their own environment conditions.Vorticity.Conformancepasses, 923 of 923. The MustNotCompile cases raise VX1010 and VX1011 under the new name.Every rename, before and now
scan.AggAsync(a => e),scan.AggAsync<TResult>(a => (…))scan.AggregateAsync(a => e),scan.AggregateAsync<TResult>(a => (…))Statisticson a scan, a projection and an aggregation,ScanStatistics,GroupStatisticsMetrics,ScanMetrics,GroupMetricssession.AppendAsync(path)session.OpenWriterAsync(path)dataset.ObjectsAsync(),RemoveAsync(objects),ReplaceAsync(removed, added)ListObjectsAsync(),RemoveObjectsAsync(objects),ReplaceObjectsAsync(removed, added)cursor.KeyCountAsync(),PrevAsync(),PrevKeyAsync(),SeekOpCountAtKeyAsync(),PreviousAsync(),PreviousKeyAsync(),SeekModeschema.TryGetField(utf8, out int index)schema.IndexOf(utf8), -1 when no field matchescolumn.Canonical(),column.Storage()column.AsCanonical(),column.AsStorage()VortexFileRepair.ValidLengthAsync(path)VortexFileRepair.GetValidLengthAsync(path)MaxDecompressedSize,MaxBatchDecompressedSizeandInitialReadSizeonVortexOpenOptions,VortexLimits.DefaultMaxDecompressedSizeMaxDecompressedBytes,MaxBatchDecompressedBytes,InitialReadBytes,DefaultMaxDecompressedBytesSegmentCache.Capacity,SegmentCache.SizeCapacityBytes,HeldBytesScanOptions.Pruning,VortexWriteOptions.Statistics,CompactionSchedule.LeasesUseStatistics,WriteStatistics,UseLeasesCompactionJob.Objects,GroupKeyPlan.Values,VortexDataset.ClusteringKeyPathsObjectKeys,ValueRange,ClusteringKeyKept as it is, on purpose
RowCount,ZoneCountandEntryCountnext toRowsandEntries. Turning every count into one form would rename compound names such asBlocksPrunedorObjectsInbadly.TimeUnitkeeps its plurals (Milliseconds,Days), since a unit that counts a quantity is plural, as inTimeSpan.FromMilliseconds.CalendarUnitstays singular, since it names the periodTruncaterounds down to.VortexFileFormat.InitialReadSizeandVortexLimits.MaxPostscriptSizeare constants of the Vortex format and keep its names.Sym,ExprandDTypeare the three abbreviations left, terms of the domain a reader meets on every page.Vortexprefix stays as it is. No rule would make it regular without renaming some thirty types.Inside the library
ScanMetrics: the counters a scan adds to, whose snapshot the publicScanStatisticswas. It andDatasetScanMetricsbecameScanCountersandDatasetScanCounters, and the internal members that hold them are namedCounters, besideMetricsfor the snapshot.…Bytes, and the one error message that named the internal type now namesVortexOpenOptions.MaxDecompressedBytes.KeyCursor<TKey>.SeekAsyncismode, after its type. The internal cursors keepop.Commits
Canonical().